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


##########
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:
   Addressed in c81bd840dd — `computeSourceWindowFingerprint` now falls back to 
the authoritative source-table reference in 
`MaterializedViewDefinitionMetadata` when the task config lacks 
`sourceTableName` (the legacy DELETE shape from a pre-upgrade controller), 
failing loud only when neither the config key nor the definition znode is 
available. Covered by 
`testComputeSourceWindowFingerprintFallsBackToDefinitionForLegacyTask` 
(positive) and 
`testComputeSourceWindowFingerprintFailsLoudWithoutConfigOrDefinition` 
(negative); a sunset TODO marks the shim for removal once pre-`sourceTableName` 
controllers are out of supported rolling-upgrade paths.



##########
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:
   Addressed in c81bd840dd — `computeSourceWindowFingerprint` now falls back to 
the authoritative source-table reference in 
`MaterializedViewDefinitionMetadata` when the task config lacks 
`sourceTableName` (the legacy DELETE shape from a pre-upgrade controller), 
failing loud only when neither the config key nor the definition znode is 
available. Covered by 
`testComputeSourceWindowFingerprintFallsBackToDefinitionForLegacyTask` 
(positive) and 
`testComputeSourceWindowFingerprintFailsLoudWithoutConfigOrDefinition` 
(negative); a sunset TODO marks the shim for removal once pre-`sourceTableName` 
controllers are out of supported rolling-upgrade paths.



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