From 0301ecc2035f5f07d14cfebfdeb7ffb8cafe6aeb Mon Sep 17 00:00:00 2001 From: Vartan Babayan <v.babayan@picodata.io> Date: Thu, 17 Oct 2024 17:31:52 +0300 Subject: [PATCH] refactor: rename replicaset.replicaset_uuid -> replicaset.uuid --- src/cas.rs | 2 +- src/config.rs | 58 ++++++++++++++++-------------------- src/http_server.rs | 2 +- src/replicaset.rs | 6 ++-- src/rpc/expel.rs | 2 +- src/rpc/join.rs | 4 +-- src/rpc/update_instance.rs | 2 +- src/traft/error.rs | 4 +-- test/conftest.py | 2 +- test/int/test_basics.py | 6 ++-- test/int/test_config_file.py | 14 ++++----- test/int/test_ddl.py | 2 +- test/int/test_http_server.py | 2 +- test/int/test_joining.py | 2 +- test/int/test_plugin.py | 4 +-- test/int/test_tier.py | 10 +++---- test/pgproto/auth_test.py | 4 +-- 17 files changed, 60 insertions(+), 66 deletions(-) diff --git a/src/cas.rs b/src/cas.rs index 1777dfdb76..a94b664bd2 100644 --- a/src/cas.rs +++ b/src/cas.rs @@ -198,7 +198,7 @@ fn proc_cas_local(req: &Request) -> Result<Response> { if req.cluster_name != cluster_name { return Err(TraftError::ClusterIdMismatch { instance_cluster_name: req.cluster_name.clone(), - cluster_cluster_name: cluster_name, + cluster_name, }); } diff --git a/src/config.rs b/src/config.rs index ca73690498..208e9dd1ab 100644 --- a/src/config.rs +++ b/src/config.rs @@ -195,9 +195,9 @@ Using configuration file '{args_path}'."); }, )])); - // Same story as with `cluster.tier`, the cluster_name must be provided + // Same story as with `cluster.tier`, the cluster name must be provided // in the config file. - config.cluster.cluster_name = Some("demo".into()); + config.cluster.name = Some("demo".into()); config } @@ -258,7 +258,7 @@ Using configuration file '{args_path}'."); } if let Some(cluster_name) = args.cluster_name { - config_from_args.cluster.cluster_name = Some(cluster_name.clone()); + config_from_args.cluster.name = Some(cluster_name.clone()); config_from_args.instance.cluster_name = Some(cluster_name); } @@ -405,15 +405,15 @@ Using configuration file '{args_path}'."); } } - match (&self.cluster.cluster_name, &self.instance.cluster_name) { + match (&self.cluster.name, &self.instance.cluster_name) { (Some(from_cluster), Some(from_instance)) if from_cluster != from_instance => { return Err(Error::InvalidConfiguration(format!( - "`cluster.cluster_name` ({from_cluster}) conflicts with `instance.cluster_name` ({from_instance})", + "`cluster.name` ({from_cluster}) conflicts with `instance.cluster_name` ({from_instance})", ))); } (None, None) => { return Err(Error::invalid_configuration( - "either `cluster.cluster_name` or `instance.cluster_name` must be specified", + "either `cluster.name` or `instance.cluster_name` must be specified", )); } _ => {} @@ -464,20 +464,14 @@ Using configuration file '{args_path}'."); for (param, i) in unknown_parameters.iter().zip(1..) { _ = write!(&mut buffer, "`{}{}`", param.prefix, param.name); - if *param.name.to_string() == *"instance_id".to_string() { + // TODO: this is a temporary help message implied only for 24.6 version + if param.name == "instance_id" || param.name == "cluster_id" { _ = write!(&mut buffer, " (did you mean `name`?)"); continue; - } else if *param.name.to_string() == *"cluster_id".to_string() { - _ = write!(&mut buffer, " (did you mean `cluster_name`?)"); - continue; - } else if *param.name.to_string() == *"instance_uuid".to_string() { - _ = write!(&mut buffer, " (did you mean `uuid`?)"); - continue; - } - - if let Some(best_match) = param.best_match { + } else if let Some(best_match) = param.best_match { _ = write!(&mut buffer, " (did you mean `{best_match}`?)"); } + if i != unknown_parameters.len() { _ = write!(&mut buffer, ", "); } @@ -595,13 +589,13 @@ Using configuration file '{args_path}'."); #[inline] pub fn cluster_name(&self) -> &str { - match (&self.instance.cluster_name, &self.cluster.cluster_name) { - (Some(instance_cluster_name), Some(cluster_cluster_name)) => { - assert_eq!(instance_cluster_name, cluster_cluster_name); + match (&self.instance.cluster_name, &self.cluster.name) { + (Some(instance_cluster_name), Some(cluster_name)) => { + assert_eq!(instance_cluster_name, cluster_name); instance_cluster_name } (Some(instance_cluster_name), None) => instance_cluster_name, - (None, Some(cluster_cluster_name)) => cluster_cluster_name, + (None, Some(cluster_name)) => cluster_name, (None, None) => "demo", } } @@ -904,7 +898,7 @@ fn report_unknown_fields<'a>( #[derive(PartialEq, Default, Debug, Clone, serde::Deserialize, serde::Serialize, Introspection)] pub struct ClusterConfig { - pub cluster_name: Option<String>, + pub name: Option<String>, // Option is needed to distinguish between the following cases: // when the config was specified and tier section was not @@ -1773,7 +1767,7 @@ mod tests { let yaml = r###" cluster: - cluster_name: foobar + name: foobar tier: voter: @@ -1826,11 +1820,11 @@ instance: "###; let config = PicodataConfig::read_yaml_contents(&yaml.trim()).unwrap(); let err = config.validate_from_file().unwrap_err(); - assert_eq!(err.to_string(), "invalid configuration: either `cluster.cluster_name` or `instance.cluster_name` must be specified"); + assert_eq!(err.to_string(), "invalid configuration: either `cluster.name` or `instance.cluster_name` must be specified"); let yaml = r###" cluster: - cluster_name: foo + name: foo tier: default: instance: @@ -1838,14 +1832,14 @@ instance: "###; let config = PicodataConfig::read_yaml_contents(&yaml.trim()).unwrap(); let err = config.validate_from_file().unwrap_err(); - assert_eq!(err.to_string(), "invalid configuration: `cluster.cluster_name` (foo) conflicts with `instance.cluster_name` (bar)"); + assert_eq!(err.to_string(), "invalid configuration: `cluster.name` (foo) conflicts with `instance.cluster_name` (bar)"); } #[test] fn missing_tiers_is_not_error() { let yaml = r###" cluster: - cluster_name: test + name: test "###; let cfg = PicodataConfig::read_yaml_contents(&yaml.trim()).unwrap(); @@ -1854,7 +1848,7 @@ cluster: let yaml = r###" cluster: - cluster_name: test + name: test tier: "###; let config = PicodataConfig::read_yaml_contents(&yaml.trim()).unwrap(); @@ -1866,7 +1860,7 @@ cluster: let yaml = r###" cluster: - cluster_name: test + name: test tier: default: "###; @@ -1876,7 +1870,7 @@ cluster: let yaml = r###" cluster: default_replication_factor: 3 - cluster_name: test + name: test "###; let config = PicodataConfig::read_yaml_contents(&yaml.trim()).unwrap(); config.validate_from_file().expect(""); @@ -2251,7 +2245,7 @@ instance: "###; std::env::set_var( "PICODATA_CONFIG_PARAMETERS", - " instance.tier = ABC;cluster.cluster_name=DEF ; + " instance.tier = ABC;cluster.name=DEF ; instance.audit=audit.txt ;; ; instance.data_dir=. ;" ); @@ -2260,7 +2254,7 @@ instance: "--config-parameter", "instance. memtx . memory= 999", ]).unwrap(); assert_eq!(config.instance.tier.unwrap(), "ABC"); - assert_eq!(config.cluster.cluster_name.unwrap(), "DEF"); + assert_eq!(config.cluster.name.unwrap(), "DEF"); assert_eq!(config.instance.log.level.unwrap(), args::LogLevel::Debug); assert_eq!(config.instance.memtx.memory.unwrap().to_string(), String::from("999B")); assert_eq!(config.instance.audit.unwrap(), "audit.txt"); @@ -2280,7 +2274,7 @@ instance: "###; std::env::set_var( "PICODATA_CONFIG_PARAMETERS", - " cluster.cluster_name=DEF ; + " cluster.name=DEF ; cluster.asdfasdfbasdfbasd = " ); let e = setup_for_tests(Some(yaml), &["run"]).unwrap_err(); diff --git a/src/http_server.rs b/src/http_server.rs index cdfb50fbc7..7616897818 100644 --- a/src/http_server.rs +++ b/src/http_server.rs @@ -263,7 +263,7 @@ fn get_replicasets_info( let mut tier = instance.tier.clone(); if let Some(replicaset) = replicasets.get(&replicaset_id) { is_leader = replicaset.current_master_name == instance.name; - replicaset_uuid.clone_from(&replicaset.replicaset_uuid); + replicaset_uuid.clone_from(&replicaset.uuid); debug_assert_eq!(replicaset.tier, instance.tier); tier.clone_from(&replicaset.tier); } diff --git a/src/replicaset.rs b/src/replicaset.rs index aef5492ca5..dbf4ac7103 100644 --- a/src/replicaset.rs +++ b/src/replicaset.rs @@ -23,7 +23,7 @@ pub struct Replicaset { pub replicaset_id: ReplicasetId, /// UUID used to identify replicasets by tarantool's subsystems. - pub replicaset_uuid: String, + pub uuid: String, /// Instance name of the current replication leader. pub current_master_name: InstanceName, @@ -82,7 +82,7 @@ impl Replicaset { pub fn with_one_instance(master: &Instance) -> Replicaset { Replicaset { replicaset_id: master.replicaset_id.clone(), - replicaset_uuid: master.replicaset_uuid.clone(), + uuid: master.replicaset_uuid.clone(), current_master_name: master.name.clone(), target_master_name: master.name.clone(), tier: master.tier.clone(), @@ -119,7 +119,7 @@ impl Replicaset { pub fn for_tests() -> Self { Self { replicaset_id: "r1".into(), - replicaset_uuid: "r1-uuid".into(), + uuid: "r1-uuid".into(), current_master_name: "i".into(), target_master_name: "j".into(), tier: "storage".into(), diff --git a/src/rpc/expel.rs b/src/rpc/expel.rs index 5f388a3dd0..22a2abb55a 100644 --- a/src/rpc/expel.rs +++ b/src/rpc/expel.rs @@ -30,7 +30,7 @@ crate::define_rpc_request! { if req.cluster_name != cluster_name { return Err(Error::ClusterIdMismatch { instance_cluster_name: req.cluster_name, - cluster_cluster_name: cluster_name, + cluster_name, }); } diff --git a/src/rpc/join.rs b/src/rpc/join.rs index be7e898e56..9fed4d7f5b 100644 --- a/src/rpc/join.rs +++ b/src/rpc/join.rs @@ -75,7 +75,7 @@ pub fn handle_join_request_and_wait(req: Request, timeout: Duration) -> Result<R if req.cluster_name != cluster_name { return Err(Error::ClusterIdMismatch { instance_cluster_name: req.cluster_name, - cluster_cluster_name: cluster_name, + cluster_name, }); } @@ -207,7 +207,7 @@ pub fn build_instance( if replicaset.tier != tier.name { return Err(Error::other(format!("tier mismatch: instance {instance_name} is from tier: '{}', but replicaset {replicaset_id} is from tier: '{}'", tier.name, replicaset.tier))); } - replicaset_uuid = replicaset.replicaset_uuid; + replicaset_uuid = replicaset.uuid; } else { replicaset_uuid = uuid::Uuid::new_v4().to_hyphenated().to_string(); } diff --git a/src/rpc/update_instance.rs b/src/rpc/update_instance.rs index 76f8f4a43d..6db2f548f9 100644 --- a/src/rpc/update_instance.rs +++ b/src/rpc/update_instance.rs @@ -110,7 +110,7 @@ pub fn handle_update_instance_request_and_wait(req: Request, timeout: Duration) if req.cluster_name != cluster_name { return Err(Error::ClusterIdMismatch { instance_cluster_name: req.cluster_name, - cluster_cluster_name: cluster_name, + cluster_name, }); } diff --git a/src/traft/error.rs b/src/traft/error.rs index 2c8fe63c18..fda2ad873e 100644 --- a/src/traft/error.rs +++ b/src/traft/error.rs @@ -77,10 +77,10 @@ pub enum Error { actual: &'static str, }, /// cluster_name of the joining instance mismatches the cluster_name of the cluster - #[error("cluster_name mismatch: cluster_name of the instance = {instance_cluster_name:?}, cluster_name of the cluster = {cluster_cluster_name:?}")] + #[error("cluster_name mismatch: cluster_name of the instance = {instance_cluster_name:?}, cluster_name of the cluster = {cluster_name:?}")] ClusterIdMismatch { instance_cluster_name: String, - cluster_cluster_name: String, + cluster_name: String, }, /// Instance was requested to configure replication with different replicaset. #[error("cannot replicate with different replicaset: expected {instance_rsid:?}, requested {requested_rsid:?}")] diff --git a/test/conftest.py b/test/conftest.py index 0f961e5ee0..2447cc095d 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -2343,7 +2343,7 @@ class Postgres: self.cluster.set_config_file( yaml=f""" cluster: - cluster_name: test + name: test tier: default: instance: diff --git a/test/int/test_basics.py b/test/int/test_basics.py index 0739398b05..3c14a4be42 100644 --- a/test/int/test_basics.py +++ b/test/int/test_basics.py @@ -250,7 +250,7 @@ def test_whoami_in_different_tiers(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: storage: replication_factor: 2 @@ -545,7 +545,7 @@ def test_proc_instance_info(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: storage: replication_factor: 2 @@ -611,7 +611,7 @@ def test_proc_get_vshard_config(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: storage: replication_factor: 1 diff --git a/test/int/test_config_file.py b/test/int/test_config_file.py index ca99c28637..34c521591c 100644 --- a/test/int/test_config_file.py +++ b/test/int/test_config_file.py @@ -133,7 +133,7 @@ def test_default_path_to_config_file(cluster: Cluster): f.write( """ cluster: - cluster_name: test + name: test tier: default: instance: @@ -153,7 +153,7 @@ instance: f.write( """ cluster: - cluster_name: test + name: test tier: default: instance: @@ -237,7 +237,7 @@ def test_config_file_with_garbage(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: default: replication_topology: mobius @@ -284,7 +284,7 @@ def test_config_file_box_cfg_parameters(cluster: Cluster): yaml=""" # just the required part cluster: - cluster_name: test + name: test tier: default: """ @@ -325,7 +325,7 @@ cluster: cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: default: @@ -430,7 +430,7 @@ def test_default_tier_is_not_created_with_configuration_file(cluster: Cluster): yaml=""" cluster: default_replication_factor: 3 - cluster_name: test + name: test tier: not_default: """ @@ -460,7 +460,7 @@ def test_output_config_parameters(cluster: Cluster): """ ) - output_params = """'cluster.cluster_name': + output_params = """'cluster.name': 'cluster.tier': 'cluster.default_replication_factor': 'instance.data_dir': diff --git a/test/int/test_ddl.py b/test/int/test_ddl.py index 48b79beb27..1a236a627e 100644 --- a/test/int/test_ddl.py +++ b/test/int/test_ddl.py @@ -1296,7 +1296,7 @@ def test_ddl_when_box_cfg_read_only(cluster: Cluster): yaml=""" cluster: default_replication_factor: 2 - cluster_name: test + name: test tier: default: """ diff --git a/test/int/test_http_server.py b/test/int/test_http_server.py index c539ffd95e..206fac990c 100644 --- a/test/int/test_http_server.py +++ b/test/int/test_http_server.py @@ -91,7 +91,7 @@ def test_webui_basic(instance: Instance): def test_webui_with_plugin(cluster: Cluster): cluster_cfg = """ cluster: - cluster_name: test + name: test tier: red: replication_factor: 1 diff --git a/test/int/test_joining.py b/test/int/test_joining.py index 1c0fcff993..6caabe1a68 100644 --- a/test/int/test_joining.py +++ b/test/int/test_joining.py @@ -553,7 +553,7 @@ def test_tier_mismatch_while_joining_by_the_same_replicaset_id(cluster: Cluster) cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: default: router: diff --git a/test/int/test_plugin.py b/test/int/test_plugin.py index 4e460b88f0..78119e4980 100644 --- a/test/int/test_plugin.py +++ b/test/int/test_plugin.py @@ -1528,7 +1528,7 @@ def test_set_topology(cluster: Cluster): cluster_cfg = """ cluster: - cluster_name: test + name: test tier: red: replication_factor: 1 @@ -2156,7 +2156,7 @@ def test_plugin_rpc_sdk_send_request(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: plugin_test + name: plugin_test tier: default: router: diff --git a/test/int/test_tier.py b/test/int/test_tier.py index f7352b5d87..26119c4c9b 100644 --- a/test/int/test_tier.py +++ b/test/int/test_tier.py @@ -7,7 +7,7 @@ def test_single_tier_query(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: default: router: @@ -195,7 +195,7 @@ def test_executing_query_on_instances_from_different_tiers(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: storage: replication_factor: 1 @@ -296,7 +296,7 @@ def test_vshard_configuration_on_different_tiers(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: storage: replication_factor: 2 @@ -393,7 +393,7 @@ def test_multiple_tier_and_global_tables(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: storage: replication_factor: 1 @@ -470,7 +470,7 @@ def test_tier_with_several_replicasets(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: router: replication_factor: 1 diff --git a/test/pgproto/auth_test.py b/test/pgproto/auth_test.py index ada1394e50..ad27f8cd41 100644 --- a/test/pgproto/auth_test.py +++ b/test/pgproto/auth_test.py @@ -51,7 +51,7 @@ def test_admin_auth(cluster: Cluster): cluster.set_config_file( yaml=""" cluster: - cluster_name: test + name: test tier: default: instance: @@ -87,7 +87,7 @@ def test_user_blocking_after_a_series_of_unsuccessful_auth_attempts(cluster: Clu cluster.set_config_file( yaml=f""" cluster: - cluster_id: test + name: test tier: default: instance: -- GitLab