-
Notifications
You must be signed in to change notification settings - Fork 240
[ISSUE #3503]🚀Implement General HA components and complete AutoSwitch HA implementation✨ #3504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||||||||||||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||||||||||||||||||||||
| * this work for additional information regarding copyright ownership. | ||||||||||||||||||||||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||||||||||||||||
| * (the "License"); you may not use this file except in compliance with | ||||||||||||||||||||||||||
| * the License. You may obtain a copy of the License at | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| use crate::ha::ha_client::HAClient; | ||||||||||||||||||||||||||
| use crate::ha::ha_connection_state::HAConnectionState; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| pub struct AutoSwitchHAClient; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| impl HAClient for AutoSwitchHAClient { | ||||||||||||||||||||||||||
| async fn start(&self) { | ||||||||||||||||||||||||||
| todo!() | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| async fn shutdown(&self) { | ||||||||||||||||||||||||||
| todo!() | ||||||||||||||||||||||||||
|
Comment on lines
+25
to
+29
|
||||||||||||||||||||||||||
| todo!() | |
| } | |
| async fn shutdown(&self) { | |
| todo!() | |
| // Initialize the client, e.g., set up connections or resources. | |
| println!("AutoSwitchHAClient started."); | |
| } | |
| async fn shutdown(&self) { | |
| // Release resources and close connections. | |
| println!("AutoSwitchHAClient shut down."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add documentation explaining the AutoSwitch HA strategy.
The implementation lacks documentation explaining how the AutoSwitch HA client differs from the default HA client and what auto-switching behavior it should provide.
+/// AutoSwitch HA Client that automatically switches between master nodes
+/// based on availability and performance metrics.
+///
+/// This client implements intelligent failover logic to ensure high availability
+/// of the RocketMQ broker by monitoring connection health and switching to
+/// backup masters when the primary master becomes unavailable.
pub struct AutoSwitchHAClient {
// ... fields
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In rocketmq-store/src/ha/auto_switch/auto_switch_ha_client.rs between lines 23
and 75, add documentation comments to the AutoSwitchHAClient implementation.
Explain the purpose of the AutoSwitch HA client, how it differs from the default
HA client, and describe the auto-switching behavior it provides. Place these
comments above the impl block or each method as appropriate to clarify the
design and functionality for future maintainers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace todo!() stubs with basic implementations to prevent runtime panics.
All methods currently use todo!() which will panic if called. Consider implementing basic functionality or at least safe defaults to make the code usable during development.
Here's an example of implementing some basic methods:
impl HAClient for AutoSwitchHAClient {
async fn start(&self) {
- todo!()
+ // TODO: Implement AutoSwitch HA client startup logic
+ println!("AutoSwitchHAClient: start() called");
}
async fn shutdown(&self) {
- todo!()
+ // TODO: Implement AutoSwitch HA client shutdown logic
+ println!("AutoSwitchHAClient: shutdown() called");
}
fn get_master_address(&self) -> String {
- todo!()
+ self.master_address.lock().unwrap().clone()
}
fn get_ha_master_address(&self) -> String {
- todo!()
+ self.ha_master_address.lock().unwrap().clone()
}
fn get_last_read_timestamp(&self) -> i64 {
- todo!()
+ *self.last_read_timestamp.lock().unwrap()
}
fn get_last_write_timestamp(&self) -> i64 {
- todo!()
+ *self.last_write_timestamp.lock().unwrap()
}
fn get_current_state(&self) -> HAConnectionState {
- todo!()
+ *self.current_state.lock().unwrap()
}🤖 Prompt for AI Agents
In rocketmq-store/src/ha/auto_switch/auto_switch_ha_client.rs between lines 24
and 75, all methods currently use todo!() which causes runtime panics if called.
Replace these todo!() stubs with basic implementations or safe default returns
to prevent panics during development. For async methods, provide minimal async
blocks that do nothing or log a placeholder; for getters, return default values
like empty strings or zero; for state-related methods, return a default
HAConnectionState or update an internal state variable accordingly. This will
make the code safe to call and easier to extend later.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| pub struct AutoSwitchHAConnection; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * this work for additional information regarding copyright ownership. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * (the "License"); you may not use this file except in compliance with | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * the License. You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::ha::auto_switch::auto_switch_ha_client::AutoSwitchHAClient; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::ha::default_ha_client::DefaultHAClient; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub struct GeneralHAClient { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default_ha_service: Option<DefaultHAClient>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto_switch_ha_service: Option<AutoSwitchHAClient>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix naming inconsistency between field names and types. The field names use "service" suffix but the types are "Client" types. This creates confusion and should be corrected for consistency. pub struct GeneralHAClient {
- default_ha_service: Option<DefaultHAClient>,
- auto_switch_ha_service: Option<AutoSwitchHAClient>,
+ default_ha_client: Option<DefaultHAClient>,
+ auto_switch_ha_client: Option<AutoSwitchHAClient>,
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add constructor methods and accessor functions for the struct. Similar to pub struct GeneralHAClient {
- default_ha_service: Option<DefaultHAClient>,
- auto_switch_ha_service: Option<AutoSwitchHAClient>,
+ default_ha_client: Option<DefaultHAClient>,
+ auto_switch_ha_client: Option<AutoSwitchHAClient>,
}
+
+impl GeneralHAClient {
+ pub fn new() -> Self {
+ Self {
+ default_ha_client: None,
+ auto_switch_ha_client: None,
+ }
+ }
+
+ pub fn with_default(client: DefaultHAClient) -> Self {
+ Self {
+ default_ha_client: Some(client),
+ auto_switch_ha_client: None,
+ }
+ }
+
+ pub fn with_auto_switch(client: AutoSwitchHAClient) -> Self {
+ Self {
+ default_ha_client: None,
+ auto_switch_ha_client: Some(client),
+ }
+ }
+
+ pub fn get_default_client(&self) -> Option<&DefaultHAClient> {
+ self.default_ha_client.as_ref()
+ }
+
+ pub fn get_auto_switch_client(&self) -> Option<&AutoSwitchHAClient> {
+ self.auto_switch_ha_client.as_ref()
+ }
+}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * this work for additional information regarding copyright ownership. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * (the "License"); you may not use this file except in compliance with | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * the License. You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::ha::auto_switch::auto_switch_ha_connection::AutoSwitchHAConnection; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::ha::default_ha_connection::DefaultHAConnection; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub struct GeneralHAConnection { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default_ha_connection: Option<DefaultHAConnection>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto_switch_ha_connection: Option<AutoSwitchHAConnection>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add constructor methods and accessor functions for the struct. The pub struct GeneralHAConnection {
default_ha_connection: Option<DefaultHAConnection>,
auto_switch_ha_connection: Option<AutoSwitchHAConnection>,
}
+
+impl GeneralHAConnection {
+ pub fn new() -> Self {
+ Self {
+ default_ha_connection: None,
+ auto_switch_ha_connection: None,
+ }
+ }
+
+ pub fn with_default(connection: DefaultHAConnection) -> Self {
+ Self {
+ default_ha_connection: Some(connection),
+ auto_switch_ha_connection: None,
+ }
+ }
+
+ pub fn with_auto_switch(connection: AutoSwitchHAConnection) -> Self {
+ Self {
+ default_ha_connection: None,
+ auto_switch_ha_connection: Some(connection),
+ }
+ }
+
+ pub fn get_default_connection(&self) -> Option<&DefaultHAConnection> {
+ self.default_ha_connection.as_ref()
+ }
+
+ pub fn get_auto_switch_connection(&self) -> Option<&AutoSwitchHAConnection> {
+ self.auto_switch_ha_connection.as_ref()
+ }
+}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add internal fields to support HA client functionality.
The
AutoSwitchHAClientstruct is currently empty but needs internal state to manage HA operations like master addresses, connection state, timestamps, etc.📝 Committable suggestion
🤖 Prompt for AI Agents