Skip to content

KAFKA-10543: Convert KTable joins to new PAPI#11412

Merged
vvcephei merged 16 commits intoapache:trunkfrom
jeqo:migrate-processor
Nov 8, 2021
Merged

KAFKA-10543: Convert KTable joins to new PAPI#11412
vvcephei merged 16 commits intoapache:trunkfrom
jeqo:migrate-processor

Conversation

@jeqo
Copy link
Copy Markdown
Contributor

@jeqo jeqo commented Oct 18, 2021

Changes:

  • Migrate KTable joins to new Processor API.
  • Migrate missing KTableProcessorSupplier implementations.
  • Replace KTableProcessorSupplier with new Processor API implementation.

Committer Checklist (excluded from commit message)

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

@jeqo jeqo changed the title Complete migration to new Processor API KAFKA-10543: Convert KTable joins to new PAPI Oct 19, 2021
@jeqo jeqo marked this pull request as ready for review October 19, 2021 12:05
@jeqo
Copy link
Copy Markdown
Contributor Author

jeqo commented Oct 25, 2021

cc @vvcephei

Copy link
Copy Markdown
Contributor

@vvcephei vvcephei left a comment

Choose a reason for hiding this comment

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

Thanks again for your fantastic work, @jeqo !

I just found a few commented-out lines that seem like they should be re-enabled. Otherwise, it looks good to me!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the forwarding type for a value getter should be <Void, Void>, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Interesting. As far as I can see the value getters do not forward, so retricting the types could be an option.
At the moment though, these value getters are initialized as part of the processor itself, and changing this types would break things.

Do you think we should consider redesign this internal api inthe future as well?

Comment on lines 108 to 111
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's up with this? (and below)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like this needs to be re-enabled.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

of course! 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Another disabled assertion.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And here

@jeqo jeqo force-pushed the migrate-processor branch from f29347a to 2ec187d Compare November 8, 2021 18:03
@jeqo jeqo requested a review from vvcephei November 8, 2021 18:55
Copy link
Copy Markdown
Contributor

@vvcephei vvcephei left a comment

Choose a reason for hiding this comment

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

Thanks, @jeqo !

@vvcephei vvcephei merged commit 807c5b4 into apache:trunk Nov 8, 2021
@jeqo jeqo deleted the migrate-processor branch November 9, 2021 10:56
stanislavkozlovski added a commit to stanislavkozlovski/kafka that referenced this pull request Nov 11, 2021
…ntegration-11-nov

* ak/trunk: (15 commits)
  KAFKA-13429: ignore bin on new modules (apache#11415)
  KAFKA-12648: introduce TopologyConfig and TaskConfig for topology-level overrides (apache#11272)
  KAFKA-12487: Add support for cooperative consumer protocol with sink connectors (apache#10563)
  MINOR: Log client disconnect events at INFO level (apache#11449)
  MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order (apache#11403)
  KAFKA-13417; Ensure dynamic reconfigurations set old config properly (apache#11448)
  MINOR: Adding a constant to denote UNKNOWN leader in LeaderAndEpoch (apache#11477)
  KAFKA-10543: Convert KTable joins to new PAPI (apache#11412)
  KAFKA-12226: Commit source task offsets without blocking on batch delivery (apache#11323)
  KAFKA-13396: Allow create topic without partition/replicaFactor (apache#11429)
  ...
xdgrulez pushed a commit to xdgrulez/kafka that referenced this pull request Dec 22, 2021
* Migrate KTable joins to new Processor API.
* Migrate missing KTableProcessorSupplier implementations.
* Replace KTableProcessorSupplier with new Processor API implementation.

Reviewers: John Roesler <vvcephei@apache.org>
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.

2 participants