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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 1 addition & 18 deletions src/server/cluster/cluster_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,6 @@ constexpr char kIdNotFound[] = "syncid not found";
constexpr string_view kClusterDisabled =
"Cluster is disabled. Enabled via passing --cluster_mode=emulated|yes";

ClusterShardInfos GetConfigForStats(ConnectionContext* cntx) {
CHECK(!IsClusterEmulated());
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

return config;
}

auto shards_info = config.Unwrap();
for (auto& node : shards_info) {
node.replicas.clear();
}

return shards_info;
}

} // namespace

ClusterFamily::ClusterFamily(ServerFamily* server_family) : server_family_(server_family) {
Expand Down Expand Up @@ -122,7 +105,7 @@ std::optional<ClusterShardInfos> ClusterFamily::GetShardInfos(ConnectionContext*
}

if (ClusterConfig::Current() != nullptr) {
return GetConfigForStats(cntx);
return ClusterConfig::Current()->GetConfig();
}
return nullopt;
}
Expand Down
7 changes: 4 additions & 3 deletions tests/dragonfly/cluster_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,15 +539,16 @@ async def test_cluster_managed_service_info(df_factory):
assert await c_master.execute_command("CLUSTER SHARDS") == expected_full_cluster_shards
assert await c_master_admin.execute_command("CLUSTER SHARDS") == expected_full_cluster_shards

# this flag doesn't affect cluster anymore so the results will be the same
await c_master.execute_command("config set managed_service_info true")

assert await c_master.execute_command("CLUSTER SLOTS") == expected_hidden_cluster_slots
assert await c_master.execute_command("CLUSTER SLOTS") == expected_full_cluster_slots
assert await c_master_admin.execute_command("CLUSTER SLOTS") == expected_full_cluster_slots

assert await c_master.execute_command("CLUSTER NODES") == expected_hidden_cluster_nodes
assert await c_master.execute_command("CLUSTER NODES") == expected_full_cluster_nodes
assert await c_master_admin.execute_command("CLUSTER NODES") == expected_full_cluster_nodes

assert await c_master.execute_command("CLUSTER SHARDS") == expected_hidden_cluster_shards
assert await c_master.execute_command("CLUSTER SHARDS") == expected_full_cluster_shards
assert await c_master_admin.execute_command("CLUSTER SHARDS") == expected_full_cluster_shards


Expand Down
Loading