You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/09/30 08:11:07 UTC
[GitHub] [kafka] showuon opened a new pull request #11370: MINOR: remove unneeded size and add lock coarsening to inMemoryKeyValueStore
showuon opened a new pull request #11370:
URL: https://github.com/apache/kafka/pull/11370
In https://github.com/apache/kafka/pull/7212, we reverted the change to use `TreeMap` instead of `ConcurrentSkipListMap`, but we forgot to remove the `size` computing codes.
Also, I found the `putAll` method might be time-consuming because the method itself is not locked, but it calls a locked method inside a loop. That is, if we elements we put has 1M elements, we might do lock and unlock for 1M times. Although the JVM "might" do "lock coarsening" for us, it'd better we make sure it works as what we expected.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax merged pull request #11370: MINOR: remove unneeded size and add lock coarsening to inMemoryKeyValueStore
Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #11370:
URL: https://github.com/apache/kafka/pull/11370
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #11370: MINOR: remove unneeded size and add lock coarsening to inMemoryKeyValueStore
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11370:
URL: https://github.com/apache/kafka/pull/11370#issuecomment-930998456
@mjsax @ableegoldman @guozhangwang , please help review this small PR. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #11370: MINOR: remove unneeded size and add lock coarsening to inMemoryKeyValueStore
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #11370:
URL: https://github.com/apache/kafka/pull/11370#issuecomment-937373122
Thanks for the PR @showuon! Merged to `trunk`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #11370: MINOR: remove unneeded size and add lock coarsening to inMemoryKeyValueStore
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #11370:
URL: https://github.com/apache/kafka/pull/11370#discussion_r722840836
##########
File path: streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java
##########
@@ -98,9 +96,15 @@ public synchronized void put(final Bytes key, final byte[] value) {
}
@Override
- public void putAll(final List<KeyValue<Bytes, byte[]>> entries) {
+ public synchronized void putAll(final List<KeyValue<Bytes, byte[]>> entries) {
for (final KeyValue<Bytes, byte[]> entry : entries) {
- put(entry.key, entry.value);
+ // intended to duplicate codes in #put, to avoid continuously lock/unlock cost
Review comment:
We should not duplicate code, but instead extract an internal `private` helper method `putInternal` that can be called by `put` and `putAll`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax merged pull request #11370: MINOR: remove unneeded size and add lock coarsening to inMemoryKeyValueStore
Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #11370:
URL: https://github.com/apache/kafka/pull/11370
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #11370: MINOR: remove unneeded size and add lock coarsening to inMemoryKeyValueStore
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #11370:
URL: https://github.com/apache/kafka/pull/11370#issuecomment-937373122
Thanks for the PR @showuon! Merged to `trunk`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org