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: debug compression <type> #4879

Merged
merged 2 commits into from
Apr 5, 2025
Merged

feat: debug compression <type> #4879

merged 2 commits into from
Apr 5, 2025

Conversation

romange
Copy link
Collaborator

@romange romange commented Apr 2, 2025

Extend debug compression to support value types. if no type is given, falls back to testing keys compressability.

Extend `debug compression` to support value types.
if no type is given, falls back to testing keys compressability.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
@romange romange requested a review from Copilot April 2, 2025 19:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the "debug compression" command to support value types while falling back to key compression when no type is provided.

  • Updated the Compression command interface to accept command arguments.
  • Enhanced the DoComputeHist function to process various object types (STRING, ZSET, LIST, HASH) based on an optional parameter.
  • Updated the help message and command dispatch to reflect the new argument.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/server/debugcmd.h Updated the Compression method signature to support arguments.
src/server/debugcmd.cc Modified DoComputeHist and Compression implementations to support processing different object types.
Comments suppressed due to low confidence (1)

src/server/debugcmd.cc:327

  • Consider explicitly handling cases where the provided type does not match any of the supported types (OBJ_STRING, OBJ_ZSET, OBJ_LIST, OBJ_HASH) to clarify the behavior in such scenarios.
      }

@romange romange requested a review from Copilot April 2, 2025 19:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the debug compression command to support an optional value type argument. When provided, the command computes compressibility for values of the specified type; otherwise, it falls back to testing keys’ compressibility.

  • Updated the function signature for Compression in both header and implementation files.
  • Modified the DoComputeHist function to handle an optional type parameter and dispatch accordingly.
  • Adjusted the help message for the COMPRESSION command.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/server/debugcmd.h Updated the Compression function signature to accept CmdArgList
src/server/debugcmd.cc Passed the optional type from the command, updated DoComputeHist accordingly, and improved the help message
Comments suppressed due to low confidence (3)

src/server/debugcmd.cc:297

  • When checking for the absence of a type, ensure that subsequent comparisons clearly reference the intended value. Consider explicitly dereferencing the optional when comparing it to constant values.
if (!type) {

src/server/debugcmd.cc:299

  • Ensure that comparing an optional type directly to a constant (e.g. OBJ_STRING) behaves as expected; using '*type' after confirming it has a value might improve clarity and correctness.
else if (type == OBJ_STRING && it->second.ObjType() == OBJ_STRING) {

src/server/debugcmd.cc:547

  • [nitpick] Consider merging the split help description lines into a single, coherent message or ensuring proper formatting so that users clearly understand the command usage.
"COMPRESSION [type]"

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
@romange
Copy link
Collaborator Author

romange commented Apr 4, 2025

@BorysTheDev ping

@romange romange merged commit 1bfc345 into main Apr 5, 2025
10 checks passed
@romange romange deleted the Pr3 branch April 5, 2025 08:02
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