Skip to content

Handle metrics for the same query but with different setups #3531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 14, 2025
+181 −51
Merged

Handle metrics for the same query but with different setups #3531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b3dc3c
Handle metrics for the same query but with different setups
ScottDugas Aug 5, 2025
579b363
Remove outdated TODO
ScottDugas Aug 6, 2025
a8f0776
Change QueryAndLocation back to not implementing Comparable
ScottDugas Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 37 additions & 28 deletions ...tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlExecutionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,10 @@ public synchronized PlannerMetricsProto.Info putMetrics(@Nonnull final String bl
@Nonnull final String query,
final int lineNumber,
@Nonnull final PlannerMetricsProto.Info info,
boolean isDirtyMetrics) {
return actualMetricsMap.put(new QueryAndLocation(blockName, query, lineNumber), info);
@Nonnull final List<String> setups) {
return actualMetricsMap.put(new QueryAndLocation(blockName, query, lineNumber, setups), info);
}

@Nullable
@SuppressWarnings("UnusedReturnValue")
public synchronized void markDirty() {
this.isDirtyMetrics = true;
Expand Down Expand Up @@ -388,10 +387,7 @@ public void saveMetricsAsBinaryProto() {
final var condensedMetricsMap = new LinkedHashMap<PlannerMetricsProto.Identifier, PlannerMetricsProto.Info>();
for (final var entry : actualMetricsMap.entrySet()) {
final var queryAndLocation = entry.getKey();
final var identifier = PlannerMetricsProto.Identifier.newBuilder()
.setBlockName(queryAndLocation.getBlockName())
.setQuery(queryAndLocation.getQuery())
.build();
final var identifier = queryAndLocation.getIdentifier();
if (condensedMetricsMap.containsKey(identifier)) {
logger.warn("⚠️ Repeated query in block {} at line {}", queryAndLocation.getBlockName(),
queryAndLocation.getLineNumber());
Expand Down Expand Up @@ -423,20 +419,25 @@ public void saveMetricsAsYaml() {

final var mmap = LinkedListMultimap.<String, Map<String, Object>>create();
for (final var entry : actualMetricsMap.entrySet()) {
final var identifier = entry.getKey();
final var identifier = entry.getKey().getIdentifier();
final var info = entry.getValue();
final var countersAndTimers = info.getCountersAndTimers();
final var infoMap =
ImmutableMap.<String, Object>of("query", identifier.getQuery(),
"explain", info.getExplain(),
"task_count", countersAndTimers.getTaskCount(),
"task_total_time_ms", TimeUnit.NANOSECONDS.toMillis(countersAndTimers.getTaskTotalTimeNs()),
"transform_count", countersAndTimers.getTransformCount(),
"transform_time_ms", TimeUnit.NANOSECONDS.toMillis(countersAndTimers.getTransformTimeNs()),
"transform_yield_count", countersAndTimers.getTransformYieldCount(),
"insert_time_ms", TimeUnit.NANOSECONDS.toMillis(countersAndTimers.getInsertTimeNs()),
"insert_new_count", countersAndTimers.getInsertNewCount(),
"insert_reused_count", countersAndTimers.getInsertReusedCount());
final var infoMap = new LinkedHashMap<String, Object>();
infoMap.put("query", identifier.getQuery());
// only include setup if it is non-empty, in part so that the PR that adds setup doesn't change every
// metric in the yaml files
if (identifier.getSetupsCount() > 0) {
infoMap.put("setup", identifier.getSetupsList());
}
infoMap.put("explain", info.getExplain());
infoMap.put("task_count", countersAndTimers.getTaskCount());
infoMap.put("task_total_time_ms", TimeUnit.NANOSECONDS.toMillis(countersAndTimers.getTaskTotalTimeNs()));
infoMap.put("transform_count", countersAndTimers.getTransformCount());
infoMap.put("transform_time_ms", TimeUnit.NANOSECONDS.toMillis(countersAndTimers.getTransformTimeNs()));
infoMap.put("transform_yield_count", countersAndTimers.getTransformYieldCount());
infoMap.put("insert_time_ms", TimeUnit.NANOSECONDS.toMillis(countersAndTimers.getInsertTimeNs()));
infoMap.put("insert_new_count", countersAndTimers.getInsertNewCount());
infoMap.put("insert_reused_count", countersAndTimers.getInsertReusedCount());
mmap.put(identifier.getBlockName(), infoMap);
}

Expand Down Expand Up @@ -513,23 +514,31 @@ private static String baseName(@Nonnull final String resourcePath) {

private static class QueryAndLocation {
@Nonnull
private final String blockName;
private final String query;
private final PlannerMetricsProto.Identifier identifier;
private final int lineNumber;

public QueryAndLocation(@Nonnull final String blockName, final String query, final int lineNumber) {
this.blockName = blockName;
this.query = query;
public QueryAndLocation(@Nonnull final String blockName, final String query, final int lineNumber,
@Nonnull List<String> setups) {
identifier = PlannerMetricsProto.Identifier.newBuilder()
.setBlockName(blockName)
.setQuery(query)
.addAllSetups(setups)
.build();
this.lineNumber = lineNumber;
}

@Nonnull
public PlannerMetricsProto.Identifier getIdentifier() {
return identifier;
}

@Nonnull
public String getBlockName() {
return blockName;
return identifier.getBlockName();
}

public String getQuery() {
return query;
return identifier.getQuery();
}

public int getLineNumber() {
Expand All @@ -542,12 +551,12 @@ public boolean equals(final Object o) {
return false;
}
final QueryAndLocation that = (QueryAndLocation)o;
return lineNumber == that.lineNumber && Objects.equals(blockName, that.blockName) && Objects.equals(query, that.query);
return lineNumber == that.lineNumber && Objects.equals(identifier, that.identifier);
}

@Override
public int hashCode() {
return Objects.hash(blockName, query, lineNumber);
return Objects.hash(identifier, lineNumber);
}
}

Expand Down
48 changes: 30 additions & 18 deletions ...-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/QueryConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ String decorateQuery(@Nonnull String query) {
}

final void checkResult(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription,
@Nonnull YamlConnection connection) {
@Nonnull YamlConnection connection, @Nonnull List<String> setups) {
try {
checkResultInternal(currentQuery, actual, queryDescription);
checkResultInternal(currentQuery, actual, queryDescription, setups);
} catch (AssertionFailedError e) {
throw executionContext.wrapContext(e,
() -> "‼️Check result failed in config at line " + getLineNumber() + " against connection for versions " + connection.getVersions(),
Expand All @@ -163,7 +163,7 @@ final void checkError(@Nonnull SQLException actual, @Nonnull String queryDescrip
}

abstract void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription) throws SQLException;
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException;

void checkErrorInternal(@Nonnull SQLException e, @Nonnull String queryDescription) throws SQLException {
final var diffMessage = String.format(Locale.ROOT, "‼️ statement failed with the following error at line %s:%n" +
Expand All @@ -181,7 +181,8 @@ private static QueryConfig getCheckResultConfig(boolean isExpectedOrdered, @Null
return new QueryConfig(configName, value, lineNumber, executionContext) {

@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
logger.debug("⛳️ Matching results of query '{}'", queryDescription);
try (RelationalResultSet resultSet = (RelationalResultSet)actual) {
final var matchResult = Matchers.matchResultSet(getVal(), resultSet, isExpectedOrdered);
Expand Down Expand Up @@ -220,7 +221,8 @@ String decorateQuery(@Nonnull String query) {

@SuppressWarnings({"PMD.CloseResource", "PMD.EmptyWhileStmt"}) // lifetime of autocloseable resource persists beyond method
@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
logger.debug("⛳️ Matching plan for query '{}'", queryDescription);
final var resultSet = (RelationalResultSet) actual;
resultSet.next();
Expand All @@ -231,6 +233,7 @@ void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @
final var identifier = PlannerMetricsProto.Identifier.newBuilder()
.setBlockName(blockName)
.setQuery(currentQuery)
.addAllSetups(setups)
.build();
final var expectedPlannerMetricsInfo = metricsMap.get(identifier);

Expand Down Expand Up @@ -302,7 +305,7 @@ void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @
.setInsertReusedCount(actualPlannerMetrics.getLong(8)))
.build();
if (expectedPlannerMetricsInfo == null) {
executionContext.putMetrics(blockName, currentQuery, lineNumber, actualInfo, true);
executionContext.putMetrics(blockName, currentQuery, lineNumber, actualInfo, setups);
executionContext.markDirty();
logger.debug("⭐️ Successfully inserted new planner metrics at line {}", getLineNumber());
} else {
Expand Down Expand Up @@ -331,7 +334,7 @@ void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @
actualCountersAndTimers,
metricsDescriptor.findFieldByName("insert_reused_count"),
lineNumber);
executionContext.putMetrics(blockName, currentQuery, lineNumber, actualInfo, isDifferent);
executionContext.putMetrics(blockName, currentQuery, lineNumber, actualInfo, setups);
if (isDifferent) {
if (executionContext.shouldCorrectMetrics()) {
executionContext.markDirty();
Expand Down Expand Up @@ -364,7 +367,8 @@ private static QueryConfig getCheckErrorConfig(@Nullable Object value, int lineN
return new QueryConfig(QUERY_CONFIG_ERROR, value, lineNumber, executionContext) {

@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
Matchers.ResultSetPrettyPrinter resultSetPrettyPrinter = new Matchers.ResultSetPrettyPrinter();
if (actual instanceof ErrorCapturingResultSet) {
Matchers.printRemaining((ErrorCapturingResultSet) actual, resultSetPrettyPrinter);
Expand Down Expand Up @@ -406,7 +410,8 @@ private static QueryConfig getCheckCountConfig(@Nullable Object value, int lineN
return new QueryConfig(QUERY_CONFIG_COUNT, value, lineNumber, executionContext) {

@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) {
logger.debug("⛳️ Matching count of update query '{}'", queryDescription);
if (!Matchers.matches(getVal(), actual)) {
reportTestFailure(String.format(Locale.ROOT, "‼️ Expected count value %d, but got %d at line %d",
Expand All @@ -428,7 +433,8 @@ String decorateQuery(@Nonnull String query) {

@SuppressWarnings("PMD.CloseResource") // lifetime of autocloseable persists beyond method
@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
logger.debug("⛳️ Matching plan hash of query '{}'", queryDescription);
final var resultSet = (RelationalResultSet) actual;
resultSet.next();
Expand All @@ -445,7 +451,8 @@ public static QueryConfig getNoCheckConfig(int lineNumber, @Nonnull YamlExecutio
return new QueryConfig(QUERY_CONFIG_NO_CHECKS, null, lineNumber, executionContext) {
@SuppressWarnings("PMD.CloseResource") // lifetime of autocloseable persists beyond method
@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
if (actual instanceof RelationalResultSet) {
final var resultSet = (RelationalResultSet) actual;
// slurp
Expand All @@ -462,7 +469,8 @@ void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @
private static QueryConfig getMaxRowConfig(@Nonnull Object value, int lineNumber, @Nonnull YamlExecutionContext executionContext) {
return new QueryConfig(QUERY_CONFIG_MAX_ROWS, value, lineNumber, executionContext) {
@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
Assert.failUnchecked("No results to check on a maxRow config");
}
};
Expand All @@ -472,7 +480,7 @@ private static QueryConfig getSetupConfig(final Object value, final int lineNumb
return new QueryConfig(QUERY_CONFIG_SETUP, value, lineNumber, executionContext) {
@Override
void checkResultInternal(@Nonnull final String currentQuery, @Nonnull final Object actual,
@Nonnull final String queryDescription) throws SQLException {
@Nonnull final String queryDescription, @Nonnull List<String> setups) throws SQLException {
Assert.failUnchecked("No results to check on a setup config");
}
};
Expand All @@ -482,7 +490,8 @@ private static QueryConfig getDebuggerConfig(@Nonnull Object value, int lineNumb
return new QueryConfig(QUERY_CONFIG_DEBUGGER, DebuggerImplementation.valueOf(((String)value).toUpperCase(Locale.ROOT)),
lineNumber, executionContext) {
@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
Assert.failUnchecked("No results to check on a debugger config");
}
};
Expand All @@ -496,7 +505,8 @@ public static QueryConfig getSupportedVersionConfig(Object rawVersion, final int
}
return new QueryConfig(QUERY_CONFIG_SUPPORTED_VERSION, rawVersion, lineNumber, executionContext) {
@Override
void checkResultInternal(@Nonnull final String currentQuery, @Nonnull final Object actual, @Nonnull final String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull final String currentQuery, @Nonnull final Object actual,
@Nonnull final String queryDescription, @Nonnull final List<String> setups) throws SQLException {
// Nothing to do, this query is supported
// SupportedVersion configs are not executed
Assertions.fail("Supported version configs are not meant to be executed.");
Expand All @@ -516,7 +526,8 @@ public static QueryConfig getNoOpConfig(int lineNumber, @Nonnull YamlExecutionCo
return new QueryConfig(QUERY_CONFIG_NO_OP, null, lineNumber, executionContext) {
@SuppressWarnings("PMD.CloseResource") // lifetime of autocloseable persists beyond method
@Override
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual, @Nonnull String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull String currentQuery, @Nonnull Object actual,
@Nonnull String queryDescription, @Nonnull List<String> setups) throws SQLException {
// This should not be executed
Assertions.fail("NoOp Config should not be executed");
}
Expand Down Expand Up @@ -647,7 +658,7 @@ public SkipConfig(final String configMap, final Object value, final int lineNumb

@Override
void checkResultInternal(@Nonnull final String currentQuery, @Nonnull final Object actual,
@Nonnull final String queryDescription) throws SQLException {
@Nonnull final String queryDescription, @Nonnull final List<String> setups) throws SQLException {
Assertions.fail("Skipped config should not be executed: Line: " + getLineNumber() + " " + message);
}

Expand All @@ -673,7 +684,8 @@ public boolean shouldExecute(YamlConnection connection) {
}

@Override
void checkResultInternal(@Nonnull final String currentQuery, @Nonnull final Object actual, @Nonnull final String queryDescription) throws SQLException {
void checkResultInternal(@Nonnull final String currentQuery, @Nonnull final Object actual,
@Nonnull final String queryDescription, @Nonnull final List<String> setups) throws SQLException {
Assertions.fail("Check version config should not be executed: Line: " + getLineNumber());
}

Expand Down
Loading