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

[improve][io] KCA: option to collapse partitioned topics #19923

Merged
merged 2 commits into from
Mar 29, 2023

Conversation

dlg99
Copy link
Contributor

@dlg99 dlg99 commented Mar 24, 2023

Motivation

Work-around for #19922 for KCA-based sinks.

Modifications

Option to collapse topic name with "-partition-" and pass it to Kafka connector.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added unit tests.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: dlg99#13

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 24, 2023
@dlg99 dlg99 marked this pull request as draft March 24, 2023 22:45
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Overall LGTM

I have left some small comments


if (collapsePartitionedTopics
&& sourceRecord.getTopicName().isPresent()
&& TopicName.get(sourceRecord.getTopicName().get()).isPartitioned()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we an save some work by calling TopicName.get only once

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TopicName.get caches internally and this way I don't need to deal with "if not partitioned" case (and don't have to call it if collapsePartitionedTopics is disabled)

@dlg99 dlg99 marked this pull request as ready for review March 28, 2023 00:22
@eolivelli eolivelli merged commit 7cb48fd into apache:master Mar 29, 2023
dlg99 added a commit that referenced this pull request Mar 29, 2023
dlg99 added a commit to datastax/pulsar that referenced this pull request Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants