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

[dvc][server] Support separate inc push topic with different pubsub entries #1262

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

adamxchen
Copy link
Collaborator

@adamxchen adamxchen commented Oct 25, 2024

Background

In the effort of supporting separate inc push topic feature and persisting its state on disk, in addition, to minimize schema evolution and code changes, we decided to embed state of "separate inc push topic" into existing fields, e.g. upstreamOffsetMap of PartitionState. The upstreamOffsetMap currently has # of entries that correspond to the number of regions we've built. However the problem is the key of the map is the pubsub server url. Technically, in the same region, the pubsub server url is used for both RT and separate inc push topic so we needs to append a special suffix "_sep" as new keys to differentiate. This also implies a new cluster id, a new alias.

Summary

As describe in the background, while we have new cluster id, new alias, and new pubsub server url for separate inc push topic, in some certain circumstances, it gotta be resolved to the normal form in order for pubsub subscription.
This PR makes sure these values will be resolved as needed in the consumer thread and drainer thread.
I checked kafka urland cluster id usage in every downstream method and confirmed only these places are needed to resolve from new values to the old ones:

  1. Need to obtain KafkaConsumerService and TopicManager
  2. Heartbeat tracking
  3. RegionHybridConsumptionStats tracking
  4. Validation prior to producing to local pubsub

How was this PR tested?

Pending new unit tests.

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant