Skip to content

log: fix flakiness in TestStatusLogRedaction#92856

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
abarganier:log_redact_test_fix
Dec 2, 2022
Merged

log: fix flakiness in TestStatusLogRedaction#92856
craig[bot] merged 1 commit intocockroachdb:masterfrom
abarganier:log_redact_test_fix

Conversation

@abarganier
Copy link
Copy Markdown
Contributor

@abarganier abarganier commented Dec 1, 2022

TestStatusLogRedaction was continuously failing when run with --race.

The test searches for a specific log line in the results from two separate RPCs - LogFile() and Log(). The test consistently failed when checking the results from the Log() RPC because the endpoint has a default max number of log entries set to 1000. Frequently, the log entry that we're searching for has a line index in the range of [1500, 2000], so the test was failing to find the log entry since it was being filtered out due to the default max number of entries.

This patch simply sets the max to a high enough value where this should no longer happen. With multiple runs I never saw the total # of log lines exceed 2,500, so we set the max to 5,000 to be safe.

Release note: None

Fixes: #92789

@abarganier abarganier requested review from a team and dhartunian December 1, 2022 20:50
@abarganier abarganier requested a review from a team as a code owner December 1, 2022 20:50
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

:lgtm: thx for the quick investigation and fix!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

TestStatusLogRedaction was continuously failing when run
with `--race`.

The test searches for a specific log line in the results
from two separate RPCs - `LogFile()` and `Log()`. The
test consistently failed when checking the results from
the `Log()` RPC because the endpoint has a default max
number of log entries set to 1000. Frequently, the log
entry that we're searching for has a line index in the
range of [1500, 2000], so the test was failing to find
the log entry since it was being filtered out due to
the default max number of entries.

This patch simply sets the max to a high enough value
where this should no longer happen. With multiple runs
I never saw the total # of log lines exceed 2,500, so
we set the max to 5,000 to be safe.

Release note: None
@abarganier
Copy link
Copy Markdown
Contributor Author

(Had to remove the skip.UnderRaceWithIssue() call)

@abarganier
Copy link
Copy Markdown
Contributor Author

bors r=dhartunian

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 2, 2022

Build failed (retrying...):

@craig craig bot merged commit 865ad56 into cockroachdb:master Dec 2, 2022
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 2, 2022

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.

server: TestStatusLogRedaction is flaky

3 participants