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/05/06 19:52:09 UTC
[GitHub] [kafka] ableegoldman opened a new pull request #8626: KAFKA-9921: explicit handling of null values with retainDuplicates
ableegoldman opened a new pull request #8626:
URL: https://github.com/apache/kafka/pull/8626
In general the behavior of window stores with `retainDuplicates` is not well documented or enforced, so we should attempt to clarify things better in the javadocs and in the code itself. This explicitly skips the put/delete when the value is null and duplicates are allowed, and specifies this behavior in the docs.
Also adds in a test I left out in the earlier PR https://github.com/apache/kafka/pull/8564
----------------------------------------------------------------
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 #8626: KAFKA-9921: explicit handling of null values with retainDuplicates
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8626:
URL: https://github.com/apache/kafka/pull/8626#issuecomment-626040739
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] mjsax commented on pull request #8626: KAFKA-9921: explicit handling of null values with retainDuplicates
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8626:
URL: https://github.com/apache/kafka/pull/8626#issuecomment-626001773
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] GeorgiPetkov commented on a change in pull request #8626: KAFKA-9921: explicit handling of null values with retainDuplicates
Posted by GitBox <gi...@apache.org>.
GeorgiPetkov commented on a change in pull request #8626:
URL: https://github.com/apache/kafka/pull/8626#discussion_r422369326
##########
File path: streams/src/main/java/org/apache/kafka/streams/state/WindowStore.java
##########
@@ -57,6 +57,10 @@
/**
* Put a key-value pair into the window with given window start timestamp
+ * <p>
+ * If serialized value bytes are null it is treated as delete. Note that as an optimization, deletes will be
+ * skipped in the case of an underlying store that allows duplicates.
Review comment:
lol, I forgot to apply my own suggestion to use 'interpreted', good catch.
----------------------------------------------------------------
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 #8626: KAFKA-9921: explicit handling of null values with retainDuplicates
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8626:
URL: https://github.com/apache/kafka/pull/8626#issuecomment-626039488
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] GeorgiPetkov commented on a change in pull request #8626: KAFKA-9921: explicit handling of null values with retainDuplicates
Posted by GitBox <gi...@apache.org>.
GeorgiPetkov commented on a change in pull request #8626:
URL: https://github.com/apache/kafka/pull/8626#discussion_r422363058
##########
File path: streams/src/main/java/org/apache/kafka/streams/state/WindowStore.java
##########
@@ -57,6 +57,10 @@
/**
* Put a key-value pair into the window with given window start timestamp
+ * <p>
+ * If serialized value bytes are null it is treated as delete. Note that as an optimization, deletes will be
+ * skipped in the case of an underlying store that allows duplicates.
Review comment:
I would use the 'interpreted' instead of 'treated' to be more consistent with the rest of the javadoc (at least in this file). The behavior is not really due to an optimization and that's not really the point IMO. Also, 'retain duplicates' in the more common term instead of 'allow duplicates'. I would rephrase it to something like this:
```suggestion
* If serialized value bytes are null it is treated as delete. Note that deletes will be
* ignored in the case of an underlying store that retains duplicates.
```
----------------------------------------------------------------
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] GeorgiPetkov commented on a change in pull request #8626: KAFKA-9921: explicit handling of null values with retainDuplicates
Posted by GitBox <gi...@apache.org>.
GeorgiPetkov commented on a change in pull request #8626:
URL: https://github.com/apache/kafka/pull/8626#discussion_r422363058
##########
File path: streams/src/main/java/org/apache/kafka/streams/state/WindowStore.java
##########
@@ -57,6 +57,10 @@
/**
* Put a key-value pair into the window with given window start timestamp
+ * <p>
+ * If serialized value bytes are null it is treated as delete. Note that as an optimization, deletes will be
+ * skipped in the case of an underlying store that allows duplicates.
Review comment:
I would use the 'interpreted' instead of 'treated' to be more consistent with the rest of the javadoc (at least in this file). The behavior is not really due to an optimization and that's not really the point IMO. Also, 'retain duplicates' in the more common term instead of 'allow duplicates'. I would rephrase it to something like this:
```suggestion
* If serialized value bytes are null it is treated as delete. Note that deletes will be
* skipped in the case of an underlying store that retains duplicates.
```
----------------------------------------------------------------
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