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

changefeedccl: ensure pubsub sink emits resolved messages to all topics #110859

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

jayshrivastava
Copy link
Contributor

@jayshrivastava jayshrivastava commented Sep 18, 2023

Previously, the pubsub sink would emit one resolved message with an empty
topic, causing an error. This change ensures that it emits resolved
messages to each topic and the topic string is not empty. This behavior
aligns with what the old, now deprecated, pubsub sink.

Fixes: #110637
Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jayshrivastava jayshrivastava force-pushed the debug-pubsub-sink branch 2 times, most recently from f99b8e6 to 6654989 Compare September 18, 2023 21:42
@jayshrivastava jayshrivastava changed the title Debug pubsub sink changefeedccl: ensure pubsub sink emits resolved messages to all topics Sep 18, 2023
@jayshrivastava jayshrivastava marked this pull request as ready for review September 18, 2023 21:44
@jayshrivastava jayshrivastava requested a review from a team as a code owner September 18, 2023 21:44
@jayshrivastava jayshrivastava requested review from miretskiy and removed request for a team September 18, 2023 21:44
@miretskiy miretskiy added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Sep 18, 2023
Copy link
Contributor

@miretskiy miretskiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 6 of 6 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jayshrivastava)


pkg/ccl/changefeedccl/changefeed_processors.go line 1096 at r1 (raw file):

		cf.freqEmitResolved = emitNoResolved
	}

nit: probably not needed to be included in this pr.

Previously, the pubsub sink would emit one resolved message with an empty
topic, causing an error. This change ensures that it emits resolved
messages to each topic and the topic string is not empty. This behavior
aligns with what the old, now deprecated, pubsub sink.

Fixes: cockroachdb#110637
Release note: None
@jayshrivastava
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 19, 2023

Build succeeded:

@craig craig bot merged commit baeffe5 into cockroachdb:master Sep 19, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roachtest: cdc/pubsub-sink failed
3 participants