[GitHub] [kafka] ableegoldman commented on a change in pull request #10360: KAFKA-12508: Emit records with same value and same timestamp

[GitBox <gi...@apache.org>]

ableegoldman commented on a change in pull request #10360:
URL: https://github.com/apache/kafka/pull/10360#discussion_r598007541



##########
File path: streams/src/main/java/org/apache/kafka/streams/state/internals/ValueAndTimestampSerializer.java
##########
@@ -105,7 +105,7 @@ public void close() {
     }
 
     private static boolean timeIsDecreasing(final byte[] oldRecord, final byte[] newRecord) {
-        return extractTimestamp(newRecord) < extractTimestamp(oldRecord);
+        return extractTimestamp(newRecord) <= extractTimestamp(oldRecord);

Review comment:
       This is a bit unfortunate since it essentially breaks the emit-on-change semantics 😕  I guess it should be relatively rare for no-op updates to come in with the same timestamp, but this still seems like kind of a structural failure of Kafka Streams. We shouldn't need to assume that if we find an identical record in the state store then we have to forward it on the off-chance it was (1) actually the same record, and (2) was only partially processed when we happened across an unexpected exception.
   
   I'm not saying I have a better idea that could be implemented quickly & safely given the releases we're blocking, but we should at least file a ticket so users are aware of this flaw. Otherwise I imagine we might be getting bug reports that emit-on-change doesn't work. It's possible some of the orthogonal work that's been discussed in the past will end up fixing this on the side (eg buffering updates before commit, versioned tables, etc), whenever we finally get around to any of that




-- 
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