xiangfu0 commented on code in PR #18642:
URL: https://github.com/apache/pinot/pull/18642#discussion_r3367288865


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/materializedview/MaterializedViewTaskExecutor.java:
##########
@@ -602,19 +560,44 @@ private PartitionFingerprint 
getTaskFingerprint(Map<String, String> configs, Str
     return fingerprint;
   }
 
-  private void validateSourceFingerprintAtCommit(Map<String, String> configs, 
String tableName, long windowStartMs,
-      long windowEndMs, PartitionFingerprint taskFingerprint) {
+  /// Computes the current source-table [PartitionFingerprint] for 
`[windowStartMs, windowEndMs)`
+  /// from live ZK segment metadata.  Shared by the APPEND / OVERWRITE 
commit-time fingerprint
+  /// validation and the DELETE commit-time emptiness re-check.  Requires
+  /// [MaterializedViewTask#SOURCE_TABLE_NAME_KEY] in the task config (the 
scheduler sets it for
+  /// every task mode); fails loud if absent so a malformed / pre-upgrade task 
cannot silently
+  /// skip the validation.
+  private PartitionFingerprint computeSourceWindowFingerprint(Map<String, 
String> configs, String tableName,
+      long windowStartMs, long windowEndMs) {
     String sourceTableName = 
configs.get(MaterializedViewTask.SOURCE_TABLE_NAME_KEY);

Review Comment:
   This new hard requirement is not rolling-upgrade-safe. Older controllers 
already emit DELETE tasks without `SOURCE_TABLE_NAME_KEY`; if a minion is 
upgraded first, those legacy tasks now fail here instead of cleaning up the 
bucket. Because V1 routing still relies on `watermarkMs` rather than per-bucket 
state, that can strand below-watermark STALE partitions during a mixed-version 
rollout. This needs a backward-compatible fallback for legacy DELETE tasks, or 
an explicit controller-first rollout gate.



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

Reply via email to