Add 'use-test-checksum-in-determinism-analyzer' config property to Ve…

…rifier.
prestodb · Feb 4, 2025 · f913c56 · f913c56
1 parent a8a6ffd
commit f913c56
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 4 deletions.
diff --git a/...o-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java b/...o-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java
@@ -96,6 +96,7 @@ public abstract class AbstractVerification<B extends QueryBundle, R extends Matc
     protected final String runningMode;
     protected final boolean saveSnapshot;
     protected final boolean isExplain;
+    protected final boolean isUseTestChecksumInDeterminismAnalyzer;
     private final boolean concurrentControlAndTest;
     protected final SnapshotQueryConsumer snapshotQueryConsumer;
     protected final Map<String, SnapshotQuery> snapshotQueries;
@@ -131,6 +132,7 @@ public AbstractVerification(
         this.runningMode = verifierConfig.getRunningMode();
         this.saveSnapshot = verifierConfig.isSaveSnapshot();
         this.isExplain = verifierConfig.isExplain();
+        this.isUseTestChecksumInDeterminismAnalyzer = verifierConfig.isUseTestChecksumInDeterminismAnalyzer();
     }
 
     protected abstract B getQueryRewrite(ClusterType clusterType);

diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DataMatchResult.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DataMatchResult.java
@@ -53,6 +53,7 @@ public enum MatchType
     private final DataType dataType;
     private final MatchType matchType;
     private final Optional<ChecksumResult> controlChecksum;
+    private final Optional<ChecksumResult> testChecksum;
     private final OptionalLong controlRowCount;
     private final OptionalLong testRowCount;
     private final List<ColumnMatchResult<?>> mismatchedColumns;
@@ -61,13 +62,15 @@ public DataMatchResult(
             DataType dataType,
             MatchType matchType,
             Optional<ChecksumResult> controlChecksum,
+            Optional<ChecksumResult> testChecksum,
             OptionalLong controlRowCount,
             OptionalLong testRowCount,
             List<ColumnMatchResult<?>> mismatchedColumns)
     {
         this.dataType = requireNonNull(dataType, "data type is null");
         this.matchType = requireNonNull(matchType, "match type is null");
         this.controlChecksum = requireNonNull(controlChecksum, "controlChecksum is null");
+        this.testChecksum = requireNonNull(testChecksum, "testChecksum is null");
         this.controlRowCount = requireNonNull(controlRowCount, "controlRowCount is null");
         this.testRowCount = requireNonNull(testRowCount, "testRowCount is null");
         this.mismatchedColumns = ImmutableList.copyOf(mismatchedColumns);
@@ -114,6 +117,12 @@ public ChecksumResult getControlChecksum()
         return controlChecksum.get();
     }
 
+    public ChecksumResult getTestChecksum()
+    {
+        checkState(testChecksum.isPresent(), "testChecksum is missing");
+        return testChecksum.get();
+    }
+
     public List<ColumnMatchResult<?>> getMismatchedColumns()
     {
         return mismatchedColumns;

diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DataVerification.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DataVerification.java
@@ -132,6 +132,7 @@ public DataMatchResult verify(
                         DATA,
                         MATCH,
                         Optional.empty(),
+                        Optional.empty(),
                         OptionalLong.empty(),
                         OptionalLong.empty(),
                         ImmutableList.of());
@@ -146,7 +147,14 @@ else if (QUERY_BANK_MODE.equals(runningMode)) {
                 controlChecksumResult = ChecksumResult.fromJson(snapshotJson);
             }
             else {
-                return new DataMatchResult(DATA, SNAPSHOT_DOES_NOT_EXIST, Optional.empty(), OptionalLong.empty(), OptionalLong.empty(), Collections.emptyList());
+                return new DataMatchResult(
+                        DATA,
+                        SNAPSHOT_DOES_NOT_EXIST,
+                        Optional.empty(),
+                        Optional.empty(),
+                        OptionalLong.empty(),
+                        OptionalLong.empty(),
+                        Collections.emptyList());
             }
         }
 
@@ -161,7 +169,7 @@ else if (QUERY_BANK_MODE.equals(runningMode)) {
     @Override
     protected DeterminismAnalysisDetails analyzeDeterminism(QueryObjectBundle control, DataMatchResult matchResult)
     {
-        return determinismAnalyzer.analyze(control, matchResult.getControlChecksum());
+        return determinismAnalyzer.analyze(control, isUseTestChecksumInDeterminismAnalyzer ? matchResult.getTestChecksum() : matchResult.getControlChecksum());
     }
 
     @Override

diff --git a/...o-verifier/src/main/java/com/facebook/presto/verifier/framework/DataVerificationUtil.java b/...o-verifier/src/main/java/com/facebook/presto/verifier/framework/DataVerificationUtil.java
@@ -85,6 +85,7 @@ public static DataMatchResult match(
                     dataType,
                     SCHEMA_MISMATCH,
                     Optional.empty(),
+                    Optional.empty(),
                     OptionalLong.empty(),
                     OptionalLong.empty(),
                     ImmutableList.of());
@@ -107,6 +108,7 @@ public static DataMatchResult match(
                 dataType,
                 matchType,
                 Optional.of(controlChecksum),
+                Optional.of(testChecksum),
                 controlRowCount,
                 testRowCount,
                 mismatchedColumns);

diff --git a/...o-verifier/src/main/java/com/facebook/presto/verifier/framework/ExtendedVerification.java b/...o-verifier/src/main/java/com/facebook/presto/verifier/framework/ExtendedVerification.java
@@ -163,6 +163,7 @@ private Optional<DataMatchResult> verifyPartition(
                     PARTITION_DATA,
                     PARTITION_COUNT_MISMATCH,
                     Optional.empty(),
+                    Optional.empty(),
                     OptionalLong.of(controlPartitionChecksum.size()),
                     OptionalLong.of(testPartitionChecksum.size()),
                     ImmutableList.of()));
@@ -183,6 +184,7 @@ private Optional<DataMatchResult> verifyPartition(
                 PARTITION_DATA,
                 MATCH,
                 Optional.empty(),
+                Optional.empty(),
                 OptionalLong.of(controlPartitionChecksum.size()),
                 OptionalLong.of(testPartitionChecksum.size()),
                 ImmutableList.of()));
@@ -212,6 +214,7 @@ private Optional<DataMatchResult> verifyBucket(
                     BUCKET_DATA,
                     BUCKET_COUNT_MISMATCH,
                     Optional.empty(),
+                    Optional.empty(),
                     OptionalLong.of(controlBucketChecksum.size()),
                     OptionalLong.of(testBucketChecksum.size()),
                     ImmutableList.of()));
@@ -232,6 +235,7 @@ private Optional<DataMatchResult> verifyBucket(
                 BUCKET_DATA,
                 MATCH,
                 Optional.empty(),
+                Optional.empty(),
                 OptionalLong.of(controlBucketChecksum.size()),
                 OptionalLong.of(testBucketChecksum.size()),
                 ImmutableList.of()));

diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/VerifierConfig.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/VerifierConfig.java
@@ -61,6 +61,7 @@ public class VerifierConfig
     private boolean teardownOnMainClusters = true;
     private boolean skipControl;
     private boolean skipChecksum;
+    private boolean useTestChecksumInDeterminismAnalyzer;
     private boolean concurrentControlAndTest;
 
     private boolean explain;
@@ -314,7 +315,7 @@ public boolean isSetupOnMainClusters()
         return setupOnMainClusters;
     }
 
-    @ConfigDescription("If true, run control/test setup queries on control/test clusters. Otherwise, run setup queries on the help cluster.")
+    @ConfigDescription("If true, run control/test setup queries on control/test clusters. Otherwise, run setup queries on the help cluster.X")
     @Config("setup-on-main-clusters")
     public VerifierConfig setSetupOnMainClusters(boolean setupOnMainClusters)
     {
@@ -361,6 +362,19 @@ public boolean isSkipChecksum()
         return skipChecksum;
     }
 
+    @ConfigDescription("In Determinism Analyzer use test checksum, not control to determine determinism. Set to true when running helper queries on test cluster.")
+    @Config("use-test-checksum-in-determinism-analyzer")
+    public VerifierConfig setUseTestChecksumInDeterminismAnalyzer(boolean useTestChecksumInDeterminismAnalyzer)
+    {
+        this.useTestChecksumInDeterminismAnalyzer = useTestChecksumInDeterminismAnalyzer;
+        return this;
+    }
+
+    public boolean isUseTestChecksumInDeterminismAnalyzer()
+    {
+        return useTestChecksumInDeterminismAnalyzer;
+    }
+
     public boolean isExplain()
     {
         return explain;

diff --git a/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestVerifierConfig.java b/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestVerifierConfig.java
@@ -48,6 +48,7 @@ public void testDefault()
                 .setTeardownOnMainClusters(true)
                 .setSkipControl(false)
                 .setSkipChecksum(false)
+                .setUseTestChecksumInDeterminismAnalyzer(false)
                 .setExplain(false)
                 .setConcurrentControlAndTest(false)
                 .setRunningMode("control-test")
@@ -81,6 +82,7 @@ public void testExplicitPropertyMappings()
                 .put("teardown-on-main-clusters", "false")
                 .put("skip-control", "true")
                 .put("skip-checksum", "true")
+                .put("use-test-checksum-in-determinism-analyzer", "true")
                 .put("explain", "true")
                 .put("concurrent-control-and-test", "true")
                 .put("running-mode", "query-bank")
@@ -110,6 +112,7 @@ public void testExplicitPropertyMappings()
                 .setTeardownOnMainClusters(false)
                 .setSkipControl(true)
                 .setSkipChecksum(true)
+                .setUseTestChecksumInDeterminismAnalyzer(true)
                 .setExplain(true)
                 .setConcurrentControlAndTest(true)
                 .setRunningMode("query-bank")

diff --git a/...verifier/src/test/java/com/facebook/presto/verifier/resolver/FailureResolverTestUtil.java b/...verifier/src/test/java/com/facebook/presto/verifier/resolver/FailureResolverTestUtil.java
@@ -40,7 +40,7 @@ public static <T extends ColumnChecksum> ColumnMatchResult<T> createMismatchedCo
 
     public static DataMatchResult createMatchResult(ColumnMatchResult<?>... mismatchedColumns)
     {
-        return new DataMatchResult(DATA, COLUMN_MISMATCH, Optional.empty(), OptionalLong.of(1L), OptionalLong.of(1L), asList(mismatchedColumns));
+        return new DataMatchResult(DATA, COLUMN_MISMATCH, Optional.empty(), Optional.empty(), OptionalLong.of(1L), OptionalLong.of(1L), asList(mismatchedColumns));
     }
 
     public static SqlVarbinary binary(int data)