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

[KAFKA-18026] migrate KTableSource to use ProcesserSupplier#stores #17903

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

agavra
Copy link
Contributor

@agavra agavra commented Nov 21, 2024

Description

This PR is part of the implementation for KIP-1112 (KAFKA-18026). In order to have DSL operators be properly wrapped by the interface suggestion in 1112, we need to make sure they all use the ConnectedStoreProvider#stores method to connect stores instead of manually calling addStateStore.

Review Guide

  1. Checkout KTableSource which now implements the stores() method
  2. GlobalKTables no longer hack materialized by setting the source name on the builder, instead we pipe a forceQueryable into MaterializedInternal so that we directly indicate that it should be queryable/materialized instead of relying on a side effect of setting the store name
  3. StoreDelegatingProcessorSupplier is used only in the case of GlobalKTables because the stores are expected to be passed in separately in the public API -- this PR doesn't change that
  4. FactoryWrappingStoreBuilder is used so that already created store factories can be typecast as StoreBuilders.

NOTE: For point (3), I think this might be a bug in the current implementation that we may want to change in 4.0 since it's backwards incompatible. If you pass in a ProcessorSupplier to addGlobalTable, the stores() method on that will be ignored.

Testing

This is a refactor only, there is no new behaviors.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

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.

1 participant