Skip to content

Commit

Permalink
addressing more comments related to metrics pr (#3126)
Browse files Browse the repository at this point in the history
  • Loading branch information
normen662 authored Feb 10, 2025
1 parent 73f00ab commit 83eb5ff
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ public static class YamlExecutionError extends RuntimeException {
.thenComparing(QueryAndLocation::getQuery));
if (isNightly()) {
logger.info("ℹ️ Running in the NIGHTLY context.");
if (shouldCorrectExplains() || shouldCorrectMetrics()) {
logger.error("‼️ Explain and/or planner metrics cannot be modified during nightly runs.");
Assertions.fail("‼️ Explain or planner metrics cannot be modified during nightly runs. " +
"Make sure maintenance annotations have not been checked in.");
}
logger.info("ℹ️ Number of threads to be used for parallel execution " + getNumThreads());
getNightlyRepetition().ifPresent(rep -> logger.info("ℹ️ Running with high repetition value set to " + rep));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ boolean filter(final YamlTestConfig config) {
return (boolean)config.getRunnerOptions().getOrDefault(YamlExecutionContext.OPTION_CORRECT_EXPLAIN, false) &&
(boolean)config.getRunnerOptions().getOrDefault(YamlExecutionContext.OPTION_CORRECT_METRICS, false);
}
},
/**
* Used to show the plan diffs graphically.
*/
SHOW_PLAN_ON_DIFF {
@Override
boolean filter(final YamlTestConfig config) {
return (boolean)config.getRunnerOptions().getOrDefault(YamlExecutionContext.OPTION_SHOW_PLAN_ON_DIFF, false);
}
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.apple.foundationdb.relational.yamltests.configs.ForceContinuations;
import com.apple.foundationdb.relational.yamltests.configs.JDBCInProcessConfig;
import com.apple.foundationdb.relational.yamltests.configs.MultiServerConfig;
import com.apple.foundationdb.relational.yamltests.configs.ShowPlanOnDiff;
import com.apple.foundationdb.relational.yamltests.configs.YamlTestConfig;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
Expand Down Expand Up @@ -75,7 +76,8 @@ public void beforeAll(final ExtensionContext context) throws Exception {
maintainConfigs = List.of(
new CorrectExplains(new EmbeddedConfig()),
new CorrectMetrics(new EmbeddedConfig()),
new CorrectExplainsAndMetrics(new EmbeddedConfig())
new CorrectExplainsAndMetrics(new EmbeddedConfig()),
new ShowPlanOnDiff(new EmbeddedConfig())
);
}
for (final YamlTestConfig testConfig : Iterables.concat(testConfigs, maintainConfigs)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* ForceContinuations.java
* CorrectExplains.java
*
* This source file is part of the FoundationDB open source project
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* ForceContinuations.java
* CorrectExplainsAndMetrics.java
*
* This source file is part of the FoundationDB open source project
*
Expand Down Expand Up @@ -29,7 +29,7 @@
* A configuration that runs an underlying configuration, but heals expected explains in YAML files where
* expected and actual explains differ.
* <p>
* See {@link YamlExecutionContext#OPTION_CORRECT_EXPLAIN}.
* See {@link YamlExecutionContext#OPTION_CORRECT_EXPLAIN} and {@link YamlExecutionContext#OPTION_CORRECT_METRICS}.
* </p>
*/
public class CorrectExplainsAndMetrics extends ConfigWithOptions {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* ForceContinuations.java
* CorrectMetrics.java
*
* This source file is part of the FoundationDB open source project
*
Expand Down Expand Up @@ -29,7 +29,7 @@
* A configuration that runs an underlying configuration, but heals expected explains in YAML files where
* expected and actual explains differ.
* <p>
* See {@link YamlExecutionContext#OPTION_CORRECT_EXPLAIN}.
* See {@link YamlExecutionContext#OPTION_CORRECT_METRICS}.
* </p>
*/
public class CorrectMetrics extends ConfigWithOptions {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* ShowPlanOnDiff.java
*
* This source file is part of the FoundationDB open source project
*
* Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.apple.foundationdb.relational.yamltests.configs;

import com.apple.foundationdb.relational.yamltests.YamlExecutionContext;

import javax.annotation.Nonnull;
import java.util.Map;

/**
* A configuration that runs an underlying configuration, but shows the dots of the expected and the actual plan where
* expected and actual explains differ.
* <p>
* See {@link YamlExecutionContext#OPTION_SHOW_PLAN_ON_DIFF}.
* </p>
*/
public class ShowPlanOnDiff extends ConfigWithOptions {
public ShowPlanOnDiff(@Nonnull final YamlTestConfig underlying) {
super(underlying, Map.of(YamlExecutionContext.OPTION_SHOW_PLAN_ON_DIFF, true));
}
}

0 comments on commit 83eb5ff

Please sign in to comment.