Skip to content
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

mqbcfg.xsd: Increase failover timeout to 5 minutes for proxies #264

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

kaikulimu
Copy link
Collaborator

@kaikulimu kaikulimu commented Apr 26, 2024

Looking through the code, I realize that proxies does not use the cluster definitions from clusters.json at all. If you look at the constructor of ClusterProxy, it is passed a default-initialized mqbcfg::ClusterDefinition(allocator). Therefore, it will only use the default values, which is 2 minutes in the case of failover threshold. Therefore, I am raising the default to 5 minutes to make all proxies have a failover timeout of 5 minutes.

Clusters nodes obey definitions in clusters.json, so they should not be affected by this change.

@kaikulimu kaikulimu requested a review from a team as a code owner April 26, 2024 19:15
@kaikulimu kaikulimu requested a review from pniedzielski April 26, 2024 19:16
Copy link
Collaborator

@pniedzielski pniedzielski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to the XSD file looks good.

I was a little confused by the diffs in mqbcfg_messages, but it just looks like the codegen changed where it wants to put definitions/changed to inlining more functions. That's okay with me. CI passes. I just see one place where a version string isn't being outputted; I'm not sure if this is important or not.

@@ -335,15 +272,15 @@ const int ClusterMonitorConfig::DEFAULT_INITIALIZER_MAX_TIME_MASTER = 120;

const int ClusterMonitorConfig::DEFAULT_INITIALIZER_MAX_TIME_NODE = 120;

const int ClusterMonitorConfig::DEFAULT_INITIALIZER_MAX_TIME_FAILOVER = 240;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the meat and potatoes. Everything else is noise.

@@ -6330,12 +5804,12 @@ Configuration::print(bsl::ostream& stream, int level, int spacesPerLevel) const
} // close package namespace
} // close enterprise namespace

// GENERATED BY BLP_BAS_CODEGEN_2023.10.07
// GENERATED BY @BLP_BAS_CODEGEN_VERSION@
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks suspicious.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using the newest version of BAS codegen. For some reason, this string substitution doesn't work but shouldn't be a big deal.

@kaikulimu kaikulimu merged commit 6a1f241 into bloomberg:main Apr 26, 2024
13 checks passed
@kaikulimu kaikulimu deleted the proxy-failover branch April 26, 2024 20:09
alexander-e1off pushed a commit to alexander-e1off/blazingmq that referenced this pull request Oct 24, 2024
alexander-e1off pushed a commit to alexander-e1off/blazingmq that referenced this pull request Oct 24, 2024
alexander-e1off pushed a commit to alexander-e1off/blazingmq that referenced this pull request Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants