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

reset repartition flag after map #450

Merged
merged 1 commit into from
Nov 9, 2017

Conversation

dguy
Copy link
Contributor

@dguy dguy commented Nov 9, 2017

When creating the execution plan in StructuredDataSourceNode we do a stream#map(..) to add the key to the GenericRow. This causes the repartitionRequired flag to be set to true even though we haven't changed the key. In this case we don't want a repartition as it is unnecessary and also causes issues with the stream/table join, i.e., we end up with nulls when we shouldn't.
As a temporary workaround, we use reflection to reset the repartitionRequired flag on the KStream so that joins and aggregations won't require an additional repartitioning. This can be removed once KIP-159 is in as part of AK 1.1

@dguy
Copy link
Contributor Author

dguy commented Nov 9, 2017

@bluemonk3y this seems to fix the JoinIntTest. I ran it with 100% success.

Copy link

@bluemonk3y bluemonk3y left a comment

Choose a reason for hiding this comment

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

awesome - lgtm

Copy link
Contributor

@hjafarpour hjafarpour left a comment

Choose a reason for hiding this comment

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

LGTM.

@dguy dguy merged commit 2961526 into confluentinc:4.0.x Nov 9, 2017
@dguy dguy deleted the reset-repartition-flag branch November 9, 2017 18:41
@apurvam
Copy link
Contributor

apurvam commented Nov 9, 2017

Thanks for the patch @dguy I created #453 to track the work required to remove this hack once KIP-159 lands.

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.

4 participants