-
Notifications
You must be signed in to change notification settings - Fork 702
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
Add paused_purpose to INFO CLIENTS #1564
base: unstable
Are you sure you want to change the base?
Conversation
In valkey-io#1519, we added paused_actions and paused_timeout_milliseconds, it would be helpful if we add the paused_purpose since users also want to know the purpose for the pause. Signed-off-by: Binbin <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #1564 +/- ##
============================================
+ Coverage 70.78% 70.81% +0.03%
============================================
Files 120 121 +1
Lines 65046 65149 +103
============================================
+ Hits 46045 46138 +93
- Misses 19001 19011 +10
|
Signed-off-by: Binbin <[email protected]>
Signed-off-by: Binbin <[email protected]>
@valkey-io/core-team Please vote for this, Thanks a lot |
switch (purpose) { | ||
case PAUSE_BY_CLIENT_COMMAND: | ||
return "client_command"; | ||
case PAUSE_DURING_SHUTDOWN: | ||
return "during_shutdown"; | ||
case PAUSE_DURING_FAILOVER: | ||
return "during_failover"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the INFO field, but I think the name and values are not perfect. There's something about the English language that doesn't sound right to me.
paused_purpose:client_command
paused_purpose:during_shutdown
paused_purpose:during_failover
I think the word "purpose" is not correct. Purpose is not what caused something to happen. The purpose is the end goal of some action, not the source.
To describe the source of the pause, I think "cause" or "reason" are better words. "paused_cause" or "paused_reason"?
And "during" is about time. It is the answer to when, not why. Maybe we can find a better word or just skip "during"?
How about "paused_reason:shutdown"? Or "paused_reason:shutdown_in_progress" or something like that?
In #1519, we added paused_actions and paused_timeout_milliseconds,
it would be helpful if we add the paused_purpose since users also
want to know the purpose for the pause.
Currently available options: