streamingccl: unrevert graceful cutover of stream ingestion #69262
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
I have stressraced both these tests for ~20 minutes on a gceworker and not got a failure/leaked goroutine. |
adityamaru
force-pushed
the
unrevert-gen-event
branch
from
4e0817d
to
fc16401
Compare
In cockroachdb#68918 we reverted a change that taught stream ingestion processors to wait for a cutover on losing connection to the client. This was the first part of introducing the concept of generations to c2c streaming. The change was reverted due to a leaked goroutine during stress testing. This change does not alter any of the core logic but simply makes the test more reliable by adding a `Streaming` testing knob. This allows us to intercept when the stream ingestion processor receives an Event and perform the necessaery testing. Release justification: non-production code changes. Revert a revert of previously checked in logic by fixing the testing infrastructure that was leaking the goroutine.
adityamaru
force-pushed
the
unrevert-gen-event
branch
from
fc16401
to
80f7325
Compare
pbardea
approved these changes
Aug 26, 2021
|
Unrelated bazel failure. bors r=pbardea |
|
Build succeeded: |
adityamaru
added a commit
to adityamaru/cockroach
that referenced
this pull request
Sep 21, 2021
This test does not flake anymore. There have been fixes around leaked goroutines that have been checked in in the recent past but the test mistakenly remained skipped: cockroachdb#69262 I got 3000 runs under stress. Fixes: cockroachdb#68704
craig bot
pushed a commit
that referenced
this pull request
Sep 28, 2021
70513: streamingccl: unskip TestStreamIngestionFrontierProcessor r=rhu713 a=adityamaru This test does not flake anymore. There have been fixes around leaked goroutines that have been checked in in the recent past but the test mistakenly remained skipped: #69262 I got 3000 runs under stress. Fixes: #68704 70734: colencoding: reuse scratch space when key decoding bytes and decimals r=yuzefovich a=yuzefovich When we're key decoding bytes-like columns, we need to use the scratch byte slice to decode into (in case of decimals, we might need the space temporarily). Previously, we would always allocate a new byte slice only to deep copy it later when calling `coldata.Bytes.Set`. This commit teaches the cFetcher and the relevant decoding methods to reuse the same scratch space which should reduce the memory allocations. One notable change is that now when we're calling `DecodeBytesAscending`, we have to make sure to perform a deep copy so that it is safe to reuse the returned value as the scratch space in the future. Release note: None Co-authored-by: Aditya Maru <adityamaru@gmail.com> Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #68918 we reverted
a change that taught stream ingestion processors to wait for a cutover
on losing connection to the client. This was the first part of introducing
the concept of generations to c2c streaming. The change was reverted due
to a leaked goroutine during stress testing.
This change does not alter any of the core logic but simply makes the test
more reliable by adding a
Streamingtesting knob. This allows us to interceptwhen the stream ingestion processor receives an Event and perform the necessary
testing.
Fixes: #68701
Fixes: #68795
Release justification (non-production code changes): Revert a revert
of previously checked in logic by fixing the testing infrastructure
that was leaking the goroutine.