yashmayya commented on code in PR #15919:
URL: https://github.com/apache/pinot/pull/15919#discussion_r2120111385
##########
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:
+1 on trying to avoid hacking the exception map
##########
pinot-spi/src/main/java/org/apache/pinot/spi/query/QueryThreadContext.java:
##########
@@ -415,6 +429,18 @@ public static void setQueryEngine(String queryEngine) {
get().setQueryEngine(queryEngine);
}
+ /**
+ * Returns the serviceId of the query.
+ *
+ * This is usually the id that identifies the server, broker, controller,
etc.
+ *
+ * The default value of {@code null} means the serviceId is not set.
+ * @throws IllegalStateException if the {@link QueryThreadContext} is not
initialized
+ */
+ public static String getServiceId() {
+ return get().getServiceId();
+ }
Review Comment:
Why not use the term `instanceId` directly instead? `serviceId` is confusing
because it's not a term used elsewhere in the codebase.
--
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]
