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-8802: ConcurrentSkipListMap shows performance regression in cache and in-memory store #7212

Merged
merged 2 commits into from Aug 16, 2019

Conversation

@ableegoldman
Copy link
Contributor

commented Aug 14, 2019

Reverts the TreeMap -> ConcurrentSkipListMap change that caused a performance regression in 2.3, and fixes the ConcurrentModificationException by copying (just) the key set to iterate over

vvcephei added a commit to vvcephei/kafka that referenced this pull request Aug 14, 2019
KAFKA-8802: ConcurrentSkipListMap shows performance regression in cac…
…he and in-memory store

* replace ConcurrentSkipListMap
* synchronize

See apache#7212

@bbejeck bbejeck added the streams label Aug 14, 2019

@ableegoldman ableegoldman force-pushed the ableegoldman:use-TreeMap branch to 66ca00b Aug 15, 2019

@bbejeck

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

@ableegoldman some of the failures are related ConcurrentModifcationException

@ableegoldman ableegoldman changed the title KAFKA-8802: ConcurrentSkipListMap shows performance regression in cache and in-memory store [DO NOT MERGE] KAFKA-8802: ConcurrentSkipListMap shows performance regression in cache and in-memory store Aug 16, 2019

@bbejeck
Copy link
Contributor

left a comment

Thanks for the fix @ableegoldman, the changes LGTM just need to update the tests.

@ableegoldman ableegoldman changed the title [DO NOT MERGE] KAFKA-8802: ConcurrentSkipListMap shows performance regression in cache and in-memory store KAFKA-8802: ConcurrentSkipListMap shows performance regression in cache and in-memory store Aug 16, 2019

@ableegoldman

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

@ableegoldman

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

Java 11/12 failed with flaky org.apache.kafka.streams.integration.GlobalThreadShutDownOrderTest.shouldFinishGlobalStoreOperationOnShutDown (passed locally 10/10 times, seems unrelated)

Java 8 failed with org.apache.kafka.connect.integration.RebalanceSourceConnectorsIntegrationTest.testReconfigConnector

Java 11/13 passed. Retest this, please

@guozhangwang guozhangwang merged commit e2d16b5 into apache:trunk Aug 16, 2019

1 of 3 checks passed

JDK 11 and Scala 2.12 FAILURE 11820 tests run, 77 skipped, 1 failed.
Details
JDK 8 and Scala 2.11 FAILURE 11820 tests run, 77 skipped, 3 failed.
Details
JDK 11 and Scala 2.13 SUCCESS 11820 tests run, 77 skipped, 0 failed.
Details
guozhangwang added a commit that referenced this pull request Aug 16, 2019
KAFKA-8802: ConcurrentSkipListMap shows performance regression in cac…
…he and in-memory store (#7212)

Reverts the TreeMap -> ConcurrentSkipListMap change that caused a performance regression in 2.3, and fixes the ConcurrentModificationException by copying (just) the key set to iterate over

Reviewers: Bill Bejeck <bill@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
@guozhangwang

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

Cherry-picked to 2.3

ableegoldman added a commit to confluentinc/kafka that referenced this pull request Aug 20, 2019
KAFKA-8802: ConcurrentSkipListMap shows performance regression in cac…
…he and in-memory store (apache#7212)

Reverts the TreeMap -> ConcurrentSkipListMap change that caused a performance regression in 2.3, and fixes the ConcurrentModificationException by copying (just) the key set to iterate over

Reviewers: Bill Bejeck <bill@confluent.io>, Guozhang Wang <wangguoz@gmail.com>

@ableegoldman ableegoldman deleted the ableegoldman:use-TreeMap branch Aug 22, 2019

xiowu0 added a commit to linkedin/kafka that referenced this pull request Aug 22, 2019
[LI-CHERRY-PICK] [051d290] KAFKA-8802: ConcurrentSkipListMap shows pe…
…rformance regression in cache and in-memory store (apache#7212)

TICKET = KAFKA-8802
LI_DESCRIPTION =
EXIT_CRITERIA = HASH [051d290]
ORIGINAL_DESCRIPTION =

Reverts the TreeMap -> ConcurrentSkipListMap change that caused a performance regression in 2.3, and fixes the ConcurrentModificationException by copying (just) the key set to iterate over

Reviewers: Bill Bejeck <bill@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
(cherry picked from commit 051d290)
guozhangwang added a commit to confluentinc/kafka that referenced this pull request Aug 29, 2019
KAFKA-8802: ConcurrentSkipListMap shows performance regression in cac…
…he and in-memory store (apache#7212)

Reverts the TreeMap -> ConcurrentSkipListMap change that caused a performance regression in 2.3, and fixes the ConcurrentModificationException by copying (just) the key set to iterate over

Reviewers: Bill Bejeck <bill@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.