Set timestamps in last-event-id +1 millisecond to avoid duplicate messages on reconnect

ottomata · ottomata · commit da733dda63c2 · 2018-08-01T13:59:51.000-04:00
Summary: Bug: T199433 Reviewers: Ottomata, Pchelolo Reviewed By: Ottomata, Pchelolo Differential Revision: https://phabricator.wikimedia.org/D1091
diff --git a/lib/KafkaSSE.js b/lib/KafkaSSE.js
@@ -574,7 +574,10 @@ class KafkaSSE {
     /**
      * Given a kafkaMessage consumed from node-rdkafka KafkaConsumer,
      * This will update the this.latestOffsetsMap with the kafkaMessage's
-     * topic, partition, and offset + 1.
+     * topic, partition, and offset|timestamp + 1.
+     *
+     * NOTE: + 1 is necessary for Last-Event-ID for EventSource auto-resuming on reconnect.
+     * Without the + 1, the previously consumed message will be reconsumed.
      *
      * @param {KafkaMessage} kafkaMessage
      */
@@ -591,12 +594,9 @@ class KafkaSSE {
 
         // Set either timestamp or offset, depending on the configuration of useTimestampForId
         if (this.options.useTimestampForId) {
-            this.latestOffsetsMap[`${kafkaMessage.topic}/${kafkaMessage.partition}`].timestamp = kafkaMessage.timestamp;
+            this.latestOffsetsMap[`${kafkaMessage.topic}/${kafkaMessage.partition}`].timestamp = kafkaMessage.timestamp + 1;
         }
         else {
-            // TODO: Should we send offset + 1 or just offset?  +1 is the easiest way to avoid
-            // duplicates during auto-resume.  But, if this message doesn't
-            // make it to the client, they will skip it during auto resume.
             this.latestOffsetsMap[`${kafkaMessage.topic}/${kafkaMessage.partition}`].offset = kafkaMessage.offset + 1;
         }
 
