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.

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
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
Expand Down Expand Up @@ -107,9 +106,7 @@ public static class YamlExecutionError extends RuntimeException {
this.additionalOptions = additionalOptions;
this.connectionOptions = Options.none();
this.expectedMetricsMap = loadMetricsResource(resourcePath);
this.actualMetricsMap = new TreeMap<>(Comparator.comparing(QueryAndLocation::getLineNumber)
.thenComparing(QueryAndLocation::getBlockName)
.thenComparing(QueryAndLocation::getQuery));
this.actualMetricsMap = new TreeMap<>();
if (isNightly()) {
logger.info("ℹ️ Running in the NIGHTLY context.");
if (shouldCorrectExplains() || shouldCorrectMetrics()) {
Expand Down Expand Up @@ -179,11 +176,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 +384,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 +416,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 @@ -511,25 +509,33 @@ private static String baseName(@Nonnull final String resourcePath) {
return tokens[0];
}

private static class QueryAndLocation {
private static class QueryAndLocation implements Comparable<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 +548,25 @@ 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);
}

@Override
public int compareTo(final QueryAndLocation o) {
int lineCmp = Integer.compare(lineNumber, o.lineNumber);
if (lineCmp != 0) {
return lineCmp;
}
int blockCmp = getBlockName().compareTo(o.getBlockName());
if (blockCmp != 0) {
return blockCmp;
}
return getQuery().compareTo(o.getQuery());
}
}

Expand Down
Loading