Skip to content

Conversation

chakaz
Copy link
Contributor

@chakaz chakaz commented Dec 3, 2024

Specifically:

  • INFO REPLICATION does not list the replicas, but does still show connected_slaves
  • INFO SERVER does not show thread_count and os

Fixes #4173

Specifically:
* `INFO REPLICATION` does not list the replicas, but does still show
  `connected_slaves`
* `INFO SERVER` does not show `thread_count` and `os`

Fixes #4173
@@ -2222,10 +2225,12 @@ void ServerFamily::Info(CmdArgList args, const CommandContext& cmd_cntx) {
append("dragonfly_version", GetVersion());
append("redis_mode", GetRedisMode());
append("arch_bits", 64);
append("os", GetOSString());
if (!absl::GetFlag(FLAGS_managed_service_info)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could cache the result of GetFlag :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We shouldn't optimize for INFO command in that level, let alone this value can change via CONFIG SET.

Copy link
Contributor

@kostasrim kostasrim Dec 3, 2024

Choose a reason for hiding this comment

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

If it can change by CONFIG SET then an INFO ALL might contain some of the changes depending on the GetFlag at that point in time. In other words this is a race (not a data race but a race on what the thread will see when it polls GetFlag twice so the first if might be true while the second false)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can surely live our lives with this race 🤣

@chakaz chakaz merged commit 95f2320 into main Dec 3, 2024
9 checks passed
@chakaz chakaz deleted the chakaz/hide-info-repl branch December 3, 2024 14:09
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.

Managed mode operation
3 participants