Skip to content

KAFKA-9428: Add KeyQueryMetadata APIs to KafkaStreams#7960

Closed
vvcephei wants to merge 1 commit intoapache:trunkfrom
vvcephei:kafka-6144-key-query-metadata
Closed

KAFKA-9428: Add KeyQueryMetadata APIs to KafkaStreams#7960
vvcephei wants to merge 1 commit intoapache:trunkfrom
vvcephei:kafka-6144-key-query-metadata

Conversation

@vvcephei
Copy link
Copy Markdown
Contributor

Committer Checklist (excluded from commit message)

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

@vvcephei vvcephei changed the title [KAFKA-6144]: Add KeyQueryMetadata APIs to KafkaStreams KAFKA-9428: Add KeyQueryMetadata APIs to KafkaStreams Jan 14, 2020
@vvcephei
Copy link
Copy Markdown
Contributor Author

vvcephei commented Jan 14, 2020

@vvcephei
Copy link
Copy Markdown
Contributor Author

@vinothchandar & @brary, This is the first chunk from your PR #7868, which I reviewed and feel good about merging. Unless you have any objections, I'll go ahead and merge it once the tests (and system tests) pass.

inputValuesKeys,
storeName + "-" + streamThree,
DEFAULT_TIMEOUT_MS);
DEFAULT_TIMEOUT_MS,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we also move over the standby querying test from the original PR? I did not see any other test covering querying with standby replication.. @vvcephei ?

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.

Thanks for pointing this out, @vinothchandar .

The ability to query standbys (and hence this test) is part of the follow-up PR: #7962

Copy link
Copy Markdown
Member

@vinothchandar vinothchandar left a comment

Choose a reason for hiding this comment

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

LGTM.. matched with #7868 fully once

@@ -700,6 +711,7 @@ private Map<String, Assignment> computeNewAssignment(final Map<UUID, ClientMetad
clientMetadata,
partitionsForTask,
partitionsByHostState,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

may be rename partitionsByHostState in other places too?
cosmetic change. your call

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.

I'm going to skip this for now, since it's ready to merge.

Deprecate existing metadata query APIs in favor of new
ones that include standby hosts as well as partition
information.

Implements: KIP-535
Co-authored-by: Navinder Pal Singh Brar <navinder_brar@yahoo.com>
Reviewed-by: John Roesler <vvcephei@apache.org>
@vvcephei vvcephei closed this in 71c5729 Jan 15, 2020
@vvcephei vvcephei deleted the kafka-6144-key-query-metadata branch January 15, 2020 19:50
@mjsax mjsax added the kip Requires or implements a KIP label Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kip Requires or implements a KIP streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants