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

ui: do not report errors for ranges with untrusted raft log size #48032

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

darinpp
Copy link
Contributor

@darinpp darinpp commented Apr 24, 2020

Previosuly a raft log size that exceeded several times the truncation
threshold will result in a "Raft too large" reported for each replica even
though the Log Size is set to not be trusted.
This lead to confusion from the customers as it seemed as if there is a problem.
With this fix, the "Raft too large" is only reported if the raft log size is trusted.
In the cases that it isn't trusted, the raft log size is now displayed grayed for each replica.
This PR also removes the "Log Size Trusted?" from the range report and instead
the grayed log size is used to indicate untrusted log size.

Fixes #47569

Release note (admin ui change): Fixed a bug where "Raft log too large" was reported incorrectly for replicas for which the raft log size is not to be trusted.

@darinpp darinpp requested review from andreimatei and a team April 24, 2020 21:30
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@darinpp
Copy link
Contributor Author

darinpp commented Apr 24, 2020

Screen Shot 2020-04-24 at 2 16 56 PM

This shows the result after the changes are applied (the raft log too large limit was lowered to few bytes - temporarily to force the error to be generated). But it shows that for the case where the log size is trusted - an error is shown and that for the cases where the size is not trusted - the error isn't shows and the log size is grayed.

Copy link
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

@tbg can you remind me when the trusted bit is reset?

The new Size Trusted? line seems to be the only one with a question mark. It also seems redundant with the coloring of the previous row. This table is already quite overwhelming. Any interest in making the trusted thing a tool tip on a question mark in the greyed-out size cell? Then we could put more info in there too. Nobody understands the many peculiarities that this report can show any more; I think it'd be great to set a better precedent.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei)

@blathers-crl
Copy link

blathers-crl bot commented Apr 24, 2020

❌ The GitHub CI (Cockroach) build has failed on bbce1804.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl
Copy link

blathers-crl bot commented Apr 27, 2020

❌ The GitHub CI (Cockroach) build has failed on 34209ad1.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@andreimatei
Copy link
Contributor

andreimatei commented Apr 27, 2020 via email

@blathers-crl
Copy link

blathers-crl bot commented Apr 27, 2020

❌ The GitHub CI (Cockroach) build has failed on 56dd4694.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@darinpp darinpp force-pushed the fix_false_raft_too_large branch 2 times, most recently from 7f78509 to c6da58d Compare April 27, 2020 22:31
@blathers-crl
Copy link

blathers-crl bot commented Apr 27, 2020

❌ The GitHub CI (Cockroach) build has failed on c6da58d8.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

Previosuly a raft log size that exceeded several times the truncation
threshold will result in a "Raft too large" reported for each replica even
though the Log Size is set to not be trusted.
This lead to confusion from the customers as it seemed as if there is a problem.
With this fix, the "Raft too large" is only reported if the raft log size is trusted.
In the cases that it isn't trusted, the raft log size is now displayed grayed for each replica.
This PR also removes the "Log Size Trusted?" from the range report and instead
the grayed log size and a more detailed tooltip are used to indicate untrusted log size.

Fixes cockroachdb#47569

Release note (admin ui change): Fixed a bug where "Raft log too large" was reported incorrectly for replicas
for which the raft log size is not to be trusted.
@darinpp
Copy link
Contributor Author

darinpp commented Apr 28, 2020

bors r+

1 similar comment
@darinpp
Copy link
Contributor Author

darinpp commented Apr 28, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Apr 28, 2020

Already running a review

@darinpp
Copy link
Contributor Author

darinpp commented Apr 28, 2020

grrr

@darinpp
Copy link
Contributor Author

darinpp commented Apr 28, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Apr 28, 2020

Build succeeded

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.

Incorrect log size and "Raft log too large"
3 participants