Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

streamingccl: heartbeat persisted frontier #84286

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

samiskin
Copy link
Contributor

Resolves #84086

Previously we would forward the ingestion frontier's resolved timestamp
at the point of heartbeat, however this would result in the chance of
the protected timestamp record of the producer to exceed that of the
ingestion job's persisted frontier.

This PR uses the last persisted frontier value instead.

Release note (bug fix): The protected timestamp of the producer job is
no longer able to exceed the persisted ingestion frontier.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch from aed7c42 to 9c2ce6f Compare July 12, 2022 19:06
@samiskin samiskin marked this pull request as ready for review July 12, 2022 19:08
@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch 2 times, most recently from 2bca602 to a732e87 Compare July 12, 2022 19:12
@shermanCRL shermanCRL added the A-tenant-streaming Including cluster streaming label Jul 18, 2022
@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch 2 times, most recently from d90164a to 24b505c Compare July 20, 2022 12:27
@samiskin samiskin requested a review from miretskiy July 20, 2022 12:33
@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch 2 times, most recently from 4956065 to 3847dbc Compare July 21, 2022 18:22
@samiskin samiskin requested a review from miretskiy July 21, 2022 18:24
@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch from 3847dbc to ba3994a Compare July 21, 2022 20:01
@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch from ba3994a to 77baf46 Compare July 22, 2022 12:42
@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch from 0afb75c to 4cc4cf4 Compare July 29, 2022 15:17
@samiskin
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 29, 2022

Build failed (retrying...):

@shermanCRL shermanCRL added this to the 22.2 milestone Jul 29, 2022
@yuzefovich
Copy link
Member

Has merge conflict.

bors r-

@craig
Copy link
Contributor

craig bot commented Jul 29, 2022

Canceled.

Previously we would forward the ingestion frontier's resolved timestamp
at the point of heartbeat, however this would result in the chance of
the protected timestamp record of the producer to exceed that of the
ingestion job's persisted frontier.

This PR uses the last persisted frontier value instead.  It also moves
the highwater updating to the frontier processor.

Release note (bug fix): The protected timestamp of the producer job is
no longer able to exceed the persisted ingestion frontier.
@samiskin samiskin force-pushed the producer-pts-behind-checkpoint branch from 4cc4cf4 to 96149c2 Compare July 29, 2022 19:59
@samiskin
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 29, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Jul 29, 2022

Build succeeded:

@craig craig bot merged commit 7a3cf68 into cockroachdb:master Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tenant-streaming Including cluster streaming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

streamingccl: move protected timestamp forward after ingestion job checkpoints it
7 participants