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

feat: ignore managed_service_info flag for cluster #4803

Merged
merged 2 commits into from
Mar 20, 2025

Conversation

BorysTheDev
Copy link
Contributor

managed_service_info is ignored for the cluster because we have "hidden" health status for the replica

Copy link
Contributor

@andydunstall andydunstall left a comment

Choose a reason for hiding this comment

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

Thanks!

@BorysTheDev BorysTheDev force-pushed the ignore_managed_service_info_for_cluster branch from 06ff9fa to 60f86d1 Compare March 20, 2025 08:52
@BorysTheDev
Copy link
Contributor Author

BorysTheDev commented Mar 20, 2025

@adiholden maybe we need to change our behavior a little for privilege connection and show all replicas independently on their health status?

@@ -68,7 +68,7 @@ ClusterShardInfos GetConfigForStats(ConnectionContext* cntx) {
CHECK(ClusterConfig::Current() != nullptr);

auto config = ClusterConfig::Current()->GetConfig();
if (cntx->conn()->IsPrivileged() || !absl::GetFlag(FLAGS_managed_service_info)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So when community users run dragonfly, this flag is false by default.
In this case cluster node we will not expose the replicas? why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it will expose replicas only for admin connections

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I understand we want to ignore this flag

Copy link
Collaborator

Choose a reason for hiding this comment

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

but why do you want to expose replicas only for admin connections for community?
This is not the expected behaviour for community running dragonfly cluster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix

@BorysTheDev BorysTheDev force-pushed the ignore_managed_service_info_for_cluster branch from f398cf8 to 266ef41 Compare March 20, 2025 10:23
@BorysTheDev BorysTheDev requested a review from adiholden March 20, 2025 11:14
@BorysTheDev BorysTheDev merged commit 0c31da1 into main Mar 20, 2025
10 checks passed
@BorysTheDev BorysTheDev deleted the ignore_managed_service_info_for_cluster branch March 20, 2025 13:20
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.

3 participants