Jackie-Jiang commented on code in PR #15919:
URL: https://github.com/apache/pinot/pull/15919#discussion_r2116509753


##########
pinot-common/src/main/java/org/apache/pinot/common/datablock/ZeroCopyDataBlockSerde.java:
##########
@@ -114,11 +118,36 @@ private void serializeExceptions(DataBlock dataBlock, 
PinotOutputStream into)
       return;
     }
 
-    into.writeInt(errCodeToExceptionMap.size());
+    // We add 3 fake error codes to the map to store stage, worker and server 
ID.
+    // These fake error codes are only used in the serialized format.
+    // When deserialized, the returned datablock will not contain these fake 
error codes.
+    into.writeInt(errCodeToExceptionMap.size() + 3);
     for (Map.Entry<Integer, String> entry : errCodeToExceptionMap.entrySet()) {
       into.writeInt(entry.getKey());
       into.writeInt4String(entry.getValue());
     }
+    int stage;
+    int worker;
+    String serverId;
+    if (dataBlock instanceof MetadataBlock) {
+      MetadataBlock metablock = (MetadataBlock) dataBlock;
+      stage = metablock.getStage();
+      worker = metablock.getWorker();
+      serverId = metablock.getServerId() != null ? metablock.getServerId() : 
"";
+    } else {
+      stage = -1; // Default value when not initialized
+      worker = -1; // Default value when not initialized
+      serverId = ""; // Default value when not initialized
+    }
+    // Add fake error codes for stage, worker and server ID
+    into.writeInt(STAGE_ID_FAKE_ERROR_CODE);

Review Comment:
   What will happen when the old version server see the new version response? 
Will the result include 3 errors with fake error code?
   
   Going through the code, I feel there is a better way to handle this without 
hacking them into exception map. With the existing format, we know the length 
of the exceptions section (`_exceptionsLength`). If we append the new fields 
after the exception map, after deserializing the map we can tell whether there 
are extra info following the map based on `_exceptionsLength`. For old servers, 
given we only use `_exceptionsLength` to identify whether there is exception 
section, it should be able to handle new format.



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

To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to