8000 Log cluster state periodically to capture transient state for debuggability by hpatro · Pull Request #2011 · valkey-io/valkey · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Log cluster state periodically to capture transient state for debuggability #2011

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

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from

Conversation

hpatro
Copy link
Collaborator
@hpatro hpatro commented Apr 26, 2025

This PR logs CLUSTER INFO / CLUSTER NODES output every 5 seconds to the log file for verbose/debug loglevel mode.

Certain times few nodes are not in convergence with the entire cluster and there are no logs captured about the divergence. This logging could help us better analyze in test setup where we can aggressively log more cluster information.

…debugability

Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>
@hpatro hpatro requested review from madolson and enjoy-binbin April 26, 2025 07:14
Copy link
codecov bot commented Apr 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.99%. Comparing base (0b94ca6) to head (4e7f83c).
Report is 8 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2011      +/-   ##
============================================
- Coverage     71.01%   70.99%   -0.03%     
============================================
  Files           123      123              
  Lines         66033    66125      +92     
============================================
+ Hits          46892    46944      +52     
- Misses        19141    19181      +40     
Files with missing lines Coverage Δ
src/cluster_legacy.c 86.19% <100.00%> (+0.10%) ⬆️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sds cluster_info = genClusterInfoString();
sds cluster_nodes = clusterGenNodesDescription(NULL, 0, 0);

sds infostring = sdscatprintf(sdsempty(), "\r\n# Cluster info\r\n");
Copy link
Member

Choose a reason for hiding this comment

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

New lines break log parsing. If someone turns on for some reason, it should still be "valid" log lines. I'm OK logging the state, but I don't think it should just be verbatim the info fields.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting. So, it's an exception for crash report to have new lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@madolson and I discussed offline. We discussed we can have a single log line with information around failed nodes rather than all the nodes.

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
0