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

KAFKA-5689:Add MeteredWindowStore and refactor store hierarchy #3692

Closed
wants to merge 2 commits into from

Conversation

dguy
Copy link
Contributor

@dguy dguy commented Aug 18, 2017

Add MeteredWindowStore and ChangeLoggingWindowBytesStore.
Refactor Store hierarchy such that Metered is always the outermost store
Do serialization in MeteredWindowStore

@dguy
Copy link
Contributor Author

dguy commented Aug 18, 2017

@asfgit
Copy link

asfgit commented Aug 18, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/6873/
Test PASSed (JDK 7 and Scala 2.11).

Copy link
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

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

LGTM! Could you make a double check with jconsole on the metric names and check if the state store metrics are still correct as stated in ops.html?

underlyingIterator,
new StateSerdes<>(serdes.topic(), Serdes.Long(), serdes.valueSerde()));
return new MergedSortedCacheWindowStoreIterator(filteredCacheIterator,
underlyingIterator
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: do we need multi-lines here?

@asfgit
Copy link

asfgit commented Aug 18, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/6859/
Test PASSed (JDK 8 and Scala 2.12).

@dguy
Copy link
Contributor Author

dguy commented Aug 22, 2017

the metric names are the same as previous

@dguy
Copy link
Contributor Author

dguy commented Aug 22, 2017

merged to trunk

@asfgit asfgit closed this in ee8e934 Aug 22, 2017
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