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

Print logs when the cluster state changes to fail add cluster_fail_reason to CLUSTER INFO #1188

Open
wants to merge 4 commits into
base: unstable
Choose a base branch
from

Conversation

enjoy-binbin
Copy link
Member

This log allows us to easily distinguish between full coverage and
minority partition when the cluster fails. Sometimes it is not easy
to see the minority partition in a healthy shards (both primary and
replicas).

…changes

This log allows us to easily distinguish between full coverage and
minority partition when the cluster fails. Sometimes it is not easy
to see the minority partition in a healthy shards (both primary and
replicas).

Signed-off-by: Binbin <[email protected]>
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.

Project coverage is 70.69%. Comparing base (701ab72) to head (238f5ae).
Report is 8 commits behind head on unstable.

Files with missing lines Patch % Lines
src/cluster_legacy.c 86.36% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1188      +/-   ##
============================================
+ Coverage     70.47%   70.69%   +0.21%     
============================================
  Files           114      114              
  Lines         61799    63109    +1310     
============================================
+ Hits          43555    44616    +1061     
- Misses        18244    18493     +249     
Files with missing lines Coverage Δ
src/cluster_legacy.c 86.45% <86.36%> (+0.45%) ⬆️

... and 93 files with indirect coverage changes

src/cluster_legacy.c Outdated Show resolved Hide resolved
src/cluster_legacy.c Outdated Show resolved Hide resolved
src/cluster_legacy.c Outdated Show resolved Hide resolved
src/cluster_legacy.c Outdated Show resolved Hide resolved
src/cluster_legacy.c Outdated Show resolved Hide resolved
src/cluster_legacy.h Show resolved Hide resolved
src/cluster_legacy.c Show resolved Hide resolved
@enjoy-binbin enjoy-binbin changed the title Print logs when the cluster state changes to fail or the fail reason changes Print logs when the cluster state changes to fail add cluster_fail_reason to CLUSTER INFO Oct 22, 2024
@enjoy-binbin enjoy-binbin added major-decision-pending Major decision pending by TSC team needs-doc-pr This change needs to update a documentation page. Remove label once doc PR is open. labels Oct 22, 2024
@@ -5986,6 +6023,7 @@ sds genClusterInfoString(void) {

info = sdscatprintf(info,
"cluster_state:%s\r\n"
"cluster_fail_reason:%s\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.

I honestly would rather not add an info field. This is one of those things were if the cluster is down, you get more useful information from the other fields (cluster_slots_assigned and cluster_size) and I don't want to add yet another field to look at. It's also not necessarily one or the other, it could be both.

Copy link
Member Author

Choose a reason for hiding this comment

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

yean, this does seem useless, @PingXie thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-decision-pending Major decision pending by TSC team needs-doc-pr This change needs to update a documentation page. Remove label once doc PR is open.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants