rowflow: account for more memory usage of the row buffer#55877
rowflow: account for more memory usage of the row buffer#55877craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
I'm open to suggestions on how to actually de-flake the test without removing it, but I can't seem to figure it out. Locally the tests fails after a few hundred of iterations under stress as is. |
asubiotto
left a comment
There was a problem hiding this comment.
Thanks for looking at this. Does this also fix the race in #55848?
Could you describe the reason this fails in more detail? What does a rare case where this error does not occur look like? Even if the producer and consumer goroutines run concurrently, why does that affect the occurence of the error?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @asubiotto)
3f7cab7 to
3080ebe
Compare
|
Yeah, it does fix the race too.
Never mind, I think I figured it out here. Will kick off a nightly stress on a gceworker. |
Previously, we accounted for the memory usage of the row buffer from which the rows are pushed from in the routers only when copying the rows from the row container, but we also have another in-memory row buffer that we can move the rows from which was missing the memory accounting. This is now fixed which also deflakes the router disk spill test. Release note: None
3080ebe to
3557b32
Compare
|
No failures in 400k run in over an hour, so I assume that fixes it, RFAL. |
asubiotto
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained
|
The build is red because of a logic test flake. TFTR! bors r+ |
|
Build succeeded: |
Previously, we accounted for the memory usage of the row buffer from
which the rows are pushed from in the routers only when copying the rows
from the row container, but we also have another in-memory row buffer
that we can move the rows from which was missing the memory accounting.
This is now fixed which also deflakes the router disk spill test.
Fixes: #55848.
Release note: None