Skip to content

[Client] Set key in message metadata for batch message built with KEY_BASED batch builder - #11816

Closed
lhotari wants to merge 1 commit into
apache:masterfrom
lhotari:lh-set-key-metadata-for-batchmessage
Closed

[Client] Set key in message metadata for batch message built with KEY_BASED batch builder#11816
lhotari wants to merge 1 commit into
apache:masterfrom
lhotari:lh-set-key-metadata-for-batchmessage

Conversation

@lhotari

@lhotari lhotari commented Aug 27, 2021

Copy link
Copy Markdown
Member

Motivation

The BatcherBuilder.KEY_BASED doesn't seem to work with KEY_SHARED consumers.
When looking at the code, it can be seen that the message metadata for the batch message doesn't contain keying metadata that KEY_SHARED consumers use for selecting messages.

Modifications

@lhotari lhotari added the type/bug The PR fixed a bug or issue reported a bug label Aug 27, 2021
@lhotari lhotari added this to the 2.9.0 milestone Aug 27, 2021
@lhotari lhotari self-assigned this Aug 27, 2021
@lhotari

lhotari commented Aug 27, 2021

Copy link
Copy Markdown
Member Author

This needs tests.

@lhotari

lhotari commented Aug 27, 2021

Copy link
Copy Markdown
Member Author

@codelipenghui it looks like the required code to set orderingKey and partitionKey for the batch message got removed in PR #7416 . Was that an intentional change?

@lhotari
lhotari force-pushed the lh-set-key-metadata-for-batchmessage branch from 36c0f95 to 8f7646d Compare August 27, 2021 09:13
@lhotari
lhotari marked this pull request as ready for review August 27, 2021 09:15
@lhotari

lhotari commented Aug 27, 2021

Copy link
Copy Markdown
Member Author

actually it seems that this PR is invalid since the logic was just moved to another location in code by #7416 . I'll close this PR as invalid. I guess there would have to be some integration tests to verify the KEY_BASED batch builder issues that I'm trying to investigate.

@lhotari lhotari closed this Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant