kvfollowerreadsccl: maybe deflake TestBoundedStalenessDataDriven#157915
kvfollowerreadsccl: maybe deflake TestBoundedStalenessDataDriven#157915craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
This test determines what events occur by parsing the trace. In some cases, the
parsing it was using to determine a "local read followed by remote leaseholder
read" didn't account for changes in the potential trace messages encountered
when leader leases are enabled.
Here, I widen the scope of the trace parsing. Locally under stress this
elliminated the previously encountered failure:
```
datadriven.go:357: ... SNIP ... boundedstaleness/single_row:24: still running after 10.000889738s
... SNIP ...
boundedstaleness_test.go:405: condition failed to evaluate within 45s: from boundedstaleness_test.go:436: not yet a match, output:
1
events (1 found):
* event 1: colbatchscan trace on node_idx 2: local read
datadriven.go:343:
```
Fixes cockroachdb#154710
Release note: None
|
This change is |
arulajmani
left a comment
There was a problem hiding this comment.
@arulajmani reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @miraradeva)
miraradeva
left a comment
There was a problem hiding this comment.
@miraradeva reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
pkg/ccl/kvccl/kvfollowerreadsccl/boundedstaleness_test.go line 50 at r1 (raw file):
// fullTraceDebug is a flag that controls whether full traces are printed in the // case of some errors. const fullTraceDebug = false
This is just for debugging the test, right?
|
TFTR! I'm going to merge this, but I think we probably want to follow up more here. Relying on the trace is pretty brittle and currently we are actually failing to catch cases that violate some of the tests assumptions. For example, occasionally Goes to the leaseholder because of an uninitialised replica. We don't catch that right now only because of the loosy-goosy way that we parse the trace. |
|
bors r+ |
This test determines what events occur by parsing the trace. In some cases, the parsing it was using to determine a "local read followed by remote leaseholder read" didn't account for changes in the potential trace messages encountered when leader leases are enabled.
Here, I widen the scope of the trace parsing. Locally under stress this elliminated the previously encountered failure:
Fixes #154710
Release note: None