Skip to content

chore: Hide replicas from CLUSTER subcmds in managed mode #4174

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 24, 2024
Merged

chore: Hide replicas from CLUSTER subcmds in managed mode #4174

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7488120
chore: Hide replicas from `CLUSTER` subcmds in managed mode
Nov 24, 2024
58a383f
server.client()
Nov 24, 2024
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
13 changes: 9 additions & 4 deletions src/server/cluster/cluster_family.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ ABSL_FLAG(std::string, cluster_node_id, "",
"ID within a cluster, used for slot assignment. MUST be unique. If empty, uses master "
"replication ID (random string)");

ABSL_FLAG(bool, managed_service_info, false,
"Hides some implementation details from users when true (i.e. in managed service env)");

ABSL_DECLARE_FLAG(int32_t, port);
ABSL_DECLARE_FLAG(uint16_t, announce_port);

Expand Down Expand Up @@ -122,10 +125,12 @@ ClusterShardInfo ClusterFamily::GetEmulatedShardInfo(ConnectionContext* cntx) co

info.master = {.id = id_, .ip = preferred_endpoint, .port = preferred_port};

for (const auto& replica : server_family_->GetDflyCmd()->GetReplicasRoleInfo()) {
info.replicas.push_back({.id = replica.id,
.ip = replica.address,
.port = static_cast<uint16_t>(replica.listening_port)});
if (cntx->conn()->IsPrivileged() || !absl::GetFlag(FLAGS_managed_service_info)) {
for (const auto& replica : server_family_->GetDflyCmd()->GetReplicasRoleInfo()) {
info.replicas.push_back({.id = replica.id,
.ip = replica.address,
.port = static_cast<uint16_t>(replica.listening_port)});
}
}
} else {
// TODO: We currently don't save the master's ID in the replica
Expand Down
28 changes: 26 additions & 2 deletions tests/dragonfly/cluster_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,17 @@ def is_local_host(ip: str) -> bool:
return True


@dfly_args({"proactor_threads": 4, "cluster_mode": "emulated"})
# --managed_service_info means that Dragonfly is running in a managed service, so some details
# are hidden from users, see https://github.com/dragonflydb/dragonfly/issues/4173
@dfly_args({"proactor_threads": 4, "cluster_mode": "emulated", "managed_service_info": "true"})
async def test_emulated_cluster_with_replicas(df_factory):
master = df_factory.create(port=BASE_PORT)
master = df_factory.create(port=BASE_PORT, admin_port=BASE_PORT + 1000)
replicas = [df_factory.create(port=BASE_PORT + i, logtostdout=True) for i in range(1, 3)]

df_factory.start_all([master, *replicas])

c_master = aioredis.Redis(port=master.port)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use master.client() and master.admin_client() instead of explicit aioredis.Redis. the latter handles the closing of the connections automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
I had to make some changes to follow up on that, because the clients are configured slightly differently (utf8 decoding)

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, you get it built in.

c_master_admin = aioredis.Redis(port=master.admin_port)
master_id = (await c_master.execute_command("CLUSTER MYID")).decode("utf-8")

c_replicas = [aioredis.Redis(port=replica.port) for replica in replicas]
Expand Down Expand Up @@ -290,6 +293,13 @@ async def test_emulated_cluster_with_replicas(df_factory):
)

res = await c_master.execute_command("CLUSTER SLOTS")
assert verify_slots_result(
port=master.port,
answer=res[0],
replicas=[],
)

res = await c_master_admin.execute_command("CLUSTER SLOTS")
assert verify_slots_result(
port=master.port,
answer=res[0],
Expand All @@ -308,6 +318,20 @@ async def test_emulated_cluster_with_replicas(df_factory):
"node_id": master_id,
"slots": [["0", "16383"]],
},
}

assert await c_master_admin.execute_command("CLUSTER NODES") == {
f"127.0.0.1:{master.port}": {
"connected": True,
"epoch": "0",
"flags": "myself,master",
"last_ping_sent": "0",
"last_pong_rcvd": "0",
"master_id": "-",
"migrations": [],
"node_id": master_id,
"slots": [["0", "16383"]],
},
f"127.0.0.1:{replicas[0].port}": {
"connected": True,
"epoch": "0",
Expand Down
Loading