ajothomas commented on a change in pull request #14802:
URL: https://github.com/apache/beam/pull/14802#discussion_r631467160



##########
File path: 
runners/samza/src/main/java/org/apache/beam/runners/samza/runtime/SamzaTimerInternalsFactory.java
##########
@@ -682,16 +653,26 @@ public void encode(TimerKey<K> value, OutputStream 
outStream)
         throws CoderException, IOException {
 
       // encode the timestamp first
-      STRING_CODER.encode(value.timerId, outStream);
-      STRING_CODER.encode(value.stateNamespace.stringKey(), outStream);
+      STRING_CODER.encode(value.getTimerId(), outStream);
+      STRING_CODER.encode(value.getStateNamespace().stringKey(), outStream);
 
       if (keyCoder != null) {
-        keyCoder.encode(value.key, outStream);
+        keyCoder.encode(value.getKey(), outStream);
+      }
+
+      // encode a byte (1/0) to indicate the presence/absence of timerFamilyId
+      // We can use this approach to add additional fields in the future
+      if (!Strings.isNullOrEmpty(value.getTimerFamilyId())) {
+        BOOLEAN_CODER.encode(true, outStream);

Review comment:
       This was just for future proofing in the event we add more fields to 
`TimerKey`. `TimerFamilyId` is optional and in the case it is null/empty, I 
wanted to encode a flag to indicate the absence of a timer family id. If we add 
more fields in the future, we can easily decode in a backward compatible way 
using the flag.
   
   The only way we can probably omit encoding this boolean flag is to encode an 
empty string ("") whenever family id is not present.




-- 
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:
[email protected]


Reply via email to