streamingccl: don't complete producer job on cutover #117515
Merged
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
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
msbutler
force-pushed
the
butler-pts-c2c
branch
9 times, most recently
from
8c3a7f2 to
4440ff8
Compare
Previously on cutover, the producer job would complete, removing the producer side protected timestamp. To ensure a smooth fast failback to any timestamp greater or equal to the cutover timestamp, however, the original producer side should keep the pts around. This patch retains the producer side timestamp by delaying the completion of the producer job until `stream_replication.job_liveness_timeout` (default 3 days) elapses after the final heartbeat during the replication job. A future PR will deprecate this setting and allow users to directly set this producer side retention period. Epic: none Release note: none
msbutler
force-pushed
the
butler-pts-c2c
branch
from
4440ff8 to
6cc2185
Compare
msbutler
requested review from
adityamaru,
dt and
stevendanna
and removed request for
a team and
adityamaru
stevendanna
approved these changes
Jan 9, 2024
This seems like a long time to me to keep accumulating history on a source cluster if the dev/test/other cluster decides it is done replicating and goes off on its way (it also seems like a long time if it just vanishes, but I don't need to litigate the default here). Should we, in this success case, move the expiration to t+12h instead? |
|
The timeout is currently set by the cluster setting |
dt
approved these changes
Jan 9, 2024
|
TFTR! bors r=stevendanna |
|
Build succeeded: |
msbutler
added a commit
to msbutler/cockroach
that referenced
this pull request
Jan 11, 2024
A previous pr cockroachdb#117515 lowered the producer job liveness timout in the test to allow the producer job to succeed quickly on completion. The PR lowered it to 100ms which is too low, causing the job to fail on liveness timeouts. This patch bumps the timout to 1s, preventing the liveness timeout. Fixes cockroachdb#117605 Release note: none
craig bot
pushed a commit
that referenced
this pull request
Jan 11, 2024
117258: sql: use high priority for populating RoleMemberCache r=Xiang-Gu a=Xiang-Gu Previously, when the RoleMemberCache is invalid, it launches a new txn in a singleflight to read from `system.role_members` table to populate the cache. If, however, the original txn has previously laid a write intent on the same system table, then we end up having a deadlock: original txn waits for this new txn; this new txn waits for original txn. Fixes #117144 Release note (bug fix): Fixed a bug where concurrent GRANTs can cause deadlocks. 117571: build,bazci: add `pkg/build/engflow` package r=rail a=rickystewart ... and extract logic for extracting test results into a helper function. Release note: None Epic: CRDB-8308 117687: streamingccl: deflake TestPartitionedStreamClient r=dt a=msbutler A previous pr #117515 lowered the producer job liveness timout in the test to allow the producer job to succeed quickly on completion. The PR lowered it to 100ms which is too low, causing the job to fail on liveness timeouts. This patch bumps the timout to 1s, preventing the liveness timeout. Fixes #117605 Release note: none 117691: roachtest: pull tpc-e image from GAR r=srosenberg,renatolabs a=rail Previously, we pulled the tpc-e image from Docker Hub. Now that we move most of our CI images to GAR, this image will be pulled from GAR as well Epic: RE-539 Release note: None Co-authored-by: Xiang Gu <[email protected]> Co-authored-by: Ricky Stewart <[email protected]> Co-authored-by: Michael Butler <[email protected]> Co-authored-by: Rail Aliiev <[email protected]>
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.
Previously on cutover, the producer job would complete, removing the producer
side protected timestamp. To ensure a smooth fast failback to any timestamp
greater or equal to the cutover timestamp, however, the original producer side
should keep the pts around.
This patch retains the producer side timestamp by delaying the completion of
the producer job until
stream_replication.job_liveness_timeout(default 3days) elapses after the final heartbeat during the replication job. A future PR
will deprecate this setting and allow users to directly set this producer side
retention period.
Epic: none
Release note: none