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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
