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

chore: optimize info command #4137

Merged
merged 4 commits into from
Nov 17, 2024
Merged

chore: optimize info command #4137

merged 4 commits into from
Nov 17, 2024

Conversation

romange
Copy link
Collaborator

@romange romange commented Nov 16, 2024

Info command may have a large latency when returning all the sections.
But often a single section is required. Specifically,
SERVER and REPLICATION sections are often fetched by clients
or management components.

This PR:
1. Removes any hops for "INFO SERVER" command.
2. Removes some redundant stats.
3. Prints latency stats around GetMetrics command if it took to much.

    Info command has a large latency when returning all the sections.
    But often a single section is required. Specifically,
    SERVER and REPLICATION sections are often fetched by clients
    or management components.

    This PR:
    1. Removes any hops for "INFO SERVER" command.
    2. Removes some redundant stats.
    3. Prints latency stats around GetMetrics command if it took to much.

Signed-off-by: Roman Gershman <[email protected]>
@romange romange requested review from dranikpg and chakaz November 16, 2024 08:46
chakaz
chakaz previously approved these changes Nov 17, 2024
Copy link
Collaborator

@chakaz chakaz left a comment

Choose a reason for hiding this comment

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

Generally LGTM, see some nits though

src/server/server_family.cc Outdated Show resolved Hide resolved
src/server/server_family.cc Outdated Show resolved Hide resolved
src/server/server_family.cc Outdated Show resolved Hide resolved
src/server/server_family.cc Show resolved Hide resolved
Co-authored-by: Shahar Mike <[email protected]>
Signed-off-by: Roman Gershman <[email protected]>
Also, address comments

Signed-off-by: Roman Gershman <[email protected]>
chakaz
chakaz previously approved these changes Nov 17, 2024
@romange romange requested review from chakaz and removed request for dranikpg November 17, 2024 11:04
@romange romange merged commit 8bd2b9e into main Nov 17, 2024
12 checks passed
@romange romange deleted the OptInfo branch November 17, 2024 11:33
romange added a commit that referenced this pull request Nov 20, 2024
* chore: optimize info command

    Info command has a large latency when returning all the sections.
    But often a single section is required. Specifically,
    SERVER and REPLICATION sections are often fetched by clients
    or management components.

    This PR:
    1. Removes any hops for "INFO SERVER" command.
    2. Removes some redundant stats.
    3. Prints latency stats around GetMetrics command if it took to much.

Signed-off-by: Roman Gershman <[email protected]>

* Update src/server/server_family.cc

Co-authored-by: Shahar Mike <[email protected]>
Signed-off-by: Roman Gershman <[email protected]>

* chore: remove GetMetrics dependency from the REPLICATION section

Also, address comments

Signed-off-by: Roman Gershman <[email protected]>

* fix: clang build

---------

Signed-off-by: Roman Gershman <[email protected]>
Signed-off-by: Roman Gershman <[email protected]>
Co-authored-by: Shahar Mike <[email protected]>
Signed-off-by: Roman Gershman <[email protected]>
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