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

fix: circumvent KAFKA-10179 by forcing changelog topics for tables #5781

Merged
merged 2 commits into from
Jul 8, 2020

Conversation

agavra
Copy link
Contributor

@agavra agavra commented Jul 8, 2020

Description

Partial fix for #5673. This PR introduces an extra map phase for any new kafka streams application that reads from a table, thereby preventing the streams optimization that uses the source topic as a changelog topic and (on recovery) loads data directly from the topic into rocksDB. For more discussion and context, see #5673 and https://issues.apache.org/jira/browse/KAFKA-10179

This is only a partial fix because it means any new queries running this code will not hit the problem, it doesn't fix existing queries. To do that, we will follow up this commit with one that preemptively registers the schema of the original topic into the subject of the changelog topic.

Review Guide

NOTE: This PR is split into two commits because it requires rewriting almost all historical plans that used tables. Also, if someone can take a look at joins.json and let me know if this fixes a bug or if it introduces a bug - I feel like the behavior after this PR is the right one.

Testing done

Unit testing, QTT and manual test; see tables.json

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@agavra agavra changed the title Workaround 10179 fix: circumvent KAFKA-10179 by forcing changelog topics for tables Jul 8, 2020
@mjsax
Copy link
Member

mjsax commented Jul 8, 2020

From what I can tell, LGTM.

Copy link
Contributor

@rodesai rodesai left a comment

Choose a reason for hiding this comment

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

LGTM

@agavra agavra marked this pull request as ready for review July 8, 2020 15:50
@agavra agavra requested a review from a team as a code owner July 8, 2020 15:50
@agavra
Copy link
Contributor Author

agavra commented Jul 8, 2020

Merging assuming @rodesai meant to press the green tick

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.

3 participants