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 2020/04/27 20:52:43 UTC
[GitHub] [kafka] ableegoldman opened a new pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
ableegoldman opened a new pull request #8564:
URL: https://github.com/apache/kafka/pull/8564
These two options are essentially incompatible, as caching will do nothing to reduce downstream traffic and writes when it has to allow non-unique keys (skipping records where the value is also the same is a separate issue, see [KIP-557](https://cwiki.apache.org/confluence/display/KAFKA/KIP-557%3A+Add+emit+on+change+support+for+Kafka+Streams)). But enabling caching on a store that's configured to retain duplicates is actually more than just ineffective, and currently causes incorrect results.
We should just log a warning and disable caching whenever a store is retaining duplicates to avoid introducing a regression. Maybe when 3.0 comes around we should consider throwing an exception instead to alert the user more aggressively.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #8564:
URL: https://github.com/apache/kafka/pull/8564#discussion_r416233782
##########
File path: streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedWindowStoreBuilder.java
##########
@@ -56,6 +59,11 @@ public TimestampedWindowStoreBuilder(final WindowBytesStoreSupplier storeSupplie
store = new InMemoryTimestampedWindowStoreMarker(store);
}
}
+ if (storeSupplier.retainDuplicates()) {
Review comment:
Good point. Pushed the change
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] vvcephei commented on pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
Posted by GitBox <gi...@apache.org>.
vvcephei commented on pull request #8564:
URL: https://github.com/apache/kafka/pull/8564#issuecomment-620359460
test this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8564:
URL: https://github.com/apache/kafka/pull/8564#issuecomment-620235547
Retest this please.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] vvcephei commented on a change in pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
Posted by GitBox <gi...@apache.org>.
vvcephei commented on a change in pull request #8564:
URL: https://github.com/apache/kafka/pull/8564#discussion_r416232363
##########
File path: streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedWindowStoreBuilder.java
##########
@@ -56,6 +59,11 @@ public TimestampedWindowStoreBuilder(final WindowBytesStoreSupplier storeSupplie
store = new InMemoryTimestampedWindowStoreMarker(store);
}
}
+ if (storeSupplier.retainDuplicates()) {
Review comment:
```suggestion
if (storeSupplier.retainDuplicates() && enableCaching) {
```
Should we only log if we're changing the configured caching? (Also applies below)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] vvcephei commented on pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
Posted by GitBox <gi...@apache.org>.
vvcephei commented on pull request #8564:
URL: https://github.com/apache/kafka/pull/8564#issuecomment-620359328
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #8564:
URL: https://github.com/apache/kafka/pull/8564#issuecomment-620783397
Also cherry-picked to 2.5.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #8564: KAFKA-9921: disable caching on stores configured to retain duplicates
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8564:
URL: https://github.com/apache/kafka/pull/8564#issuecomment-620235837
Makes sense to me. +1
Curious to hear what @guozhangwang @vvcephei think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org