tibrewalpratik17 commented on code in PR #13489:
URL: https://github.com/apache/pinot/pull/13489#discussion_r1672847948
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/MinionTaskUtils.java:
##########
@@ -206,4 +183,55 @@ public static boolean
extractMinionAllowDownloadFromServer(TableConfig tableConf
}
return defaultValue;
}
+
+ /**
+ * Returns the validDocID bitmap from the server whose local segment crc
matches both crc of ZK metadata and
+ * deepstore copy (expectedCrc).
+ */
+ @Nullable
+ public static RoaringBitmap getValidDocIdFromServerMatchingCrc(String
tableNameWithType, String segmentName,
+ String validDocIdsType, MinionContext minionContext, String expectedCrc)
{
+ String clusterName = minionContext.getHelixManager().getClusterName();
+ HelixAdmin helixAdmin =
minionContext.getHelixManager().getClusterManagmentTool();
+ RoaringBitmap validDocIds = null;
+ List<String> servers = MinionTaskUtils.getServers(segmentName,
tableNameWithType, helixAdmin, clusterName);
+ for (String server : servers) {
+ InstanceConfig instanceConfig =
helixAdmin.getInstanceConfig(clusterName, server);
+ String endpoint = InstanceUtils.getServerAdminEndpoint(instanceConfig);
+
+ // We only need aggregated table size and the total number of docs/rows.
Skipping column related stats, by
+ // passing an empty list.
+ ServerSegmentMetadataReader serverSegmentMetadataReader = new
ServerSegmentMetadataReader();
+ ValidDocIdsBitmapResponse validDocIdsBitmapResponse;
+ try {
+ validDocIdsBitmapResponse =
+
serverSegmentMetadataReader.getValidDocIdsBitmapFromServer(tableNameWithType,
segmentName, endpoint,
+ validDocIdsType, 60_000);
+ } catch (Exception e) {
+ LOGGER.warn(
+ String.format("Unable to retrieve validDocIds bitmap for segment:
%s from endpoint: %s", segmentName,
+ endpoint), e);
+ continue;
+ }
+
+ // Check crc from the downloaded segment against the crc returned from
the server along with the valid doc id
+ // bitmap. If this doesn't match, this means that we are hitting the
race condition where the segment has been
+ // uploaded successfully while the server is still reloading the
segment. Reloading can take a while when the
+ // offheap upsert is used because we will need to delete & add all
primary keys.
+ // `BaseSingleSegmentConversionExecutor.executeTask()` already checks
for the crc from the task generator
+ // against the crc from the current segment zk metadata, so we don't
need to check that here.
+ String crcFromValidDocIdsBitmap =
validDocIdsBitmapResponse.getSegmentCrc();
+ if (!expectedCrc.equals(crcFromValidDocIdsBitmap)) {
+ // In this scenario, we are hitting the other replica of the segment
which did not commit to ZK or deepstore.
+ // We will skip processing this bitmap to query other server to
confirm if there is a valid matching CRC.
+ String message = String.format("CRC mismatch for segment: %s, expected
value based on task generator: %s, "
+ + "actual crc from validDocIdsBitmapResponse from endpoint %s:
%s", segmentName, expectedCrc, endpoint,
+ crcFromValidDocIdsBitmap);
+ LOGGER.warn(message);
+ continue;
+ }
+ validDocIds =
RoaringBitmapUtils.deserialize(validDocIdsBitmapResponse.getBitmap());
Review Comment:
I have added a `break` right after this which achieves the same.
--
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]
