Skip to content

Commit 0e51982

Browse files
normen662alecgrieser
normen662
and
alecgrieser
authored
revive and move PlannerRepl into its own component (#3466)
- extricates the `PlannerRepl` implementation of the `Debugger` interface into its own component. That lead to a reduction in test dependencies. - revive the REPL and integrate it into the YAML test framework: - new `@DebugPlanner` annotation to annotate the yaml test you want to debug - annotate a query with a new query config `- debugger: repl` to invoke the repl on that query; this will also cause the block to be executed serially and with only one repitition - run gradle target `:yaml-tests:runDebug` from within IntelliJ or the terminal to invoke the debugger --------- Co-authored-by: Alec Grieser <[email protected]>
1 parent 71a647e commit 0e51982

File tree

42 files changed

+682
-278
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+682
-278
lines changed

‎.idea/gradle.xml

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎fdb-record-layer-core/fdb-record-layer-core.gradle

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ plugins {
2424
apply from: rootProject.file('gradle/proto.gradle')
2525
apply from: rootProject.file('gradle/publishing.gradle')
2626

27-
configurations {
28-
repl.extendsFrom testRuntimeOnly
29-
}
30-
3127
dependencies {
3228
annotationProcessor project(':fdb-java-annotations')
3329
api project(':fdb-extensions')
@@ -45,13 +41,11 @@ dependencies {
4541
testCompileOnly(libs.bundles.test.compileOnly)
4642
testImplementation(libs.jcommander)
4743
testImplementation(libs.apache.commonsMath3)
48-
testImplementation(libs.jline)
4944
testImplementation(libs.junit.platform)
5045
testImplementation(libs.protobuf.util)
5146
testCompileOnly(libs.spotbugs.annotations)
5247

5348
testAnnotationProcessor(libs.autoService)
54-
repl(libs.junit.engine)
5549
}
5650

5751
sourceSets {
@@ -74,18 +68,6 @@ task testShadowJar(type: com.github.jengelman.gradle.plugins.shadow.tasks.Shadow
7468
exclude 'log4j.properties'
7569
}
7670

77-
task replJar(type: com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar) {
78-
archiveClassifier = 'repl'
79-
from sourceSets.main.output
80-
from sourceSets.test.output
81-
configurations = [ project.configurations.repl ]
82-
manifest {
83-
inheritFrom project.tasks.jar.manifest
84-
attributes 'Main-Class': 'com.apple.foundationdb.record.query.plan.debug.ReplRunner'
85-
}
86-
mergeServiceFiles()
87-
}
88-
8971
publishing {
9072
publications {
9173
library(MavenPublication) {

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/CascadesPlanner.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ public void execute() {
563563
@Override
564564
@Nonnull
565565
public Debugger.Event toTaskEvent(final Location location) {
566-
return new Debugger.InitiatePlannerPhaseEvent(plannerPhase, currentRoot, taskStack);
566+
return new Debugger.InitiatePlannerPhaseEvent(plannerPhase, currentRoot, taskStack, location);
567567
}
568568

569569
@Override

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/debug/Debugger.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,9 @@ public PEvent.Builder toEventBuilder() {
535535
class InitiatePlannerPhaseEvent extends AbstractEventWithState {
536536
public InitiatePlannerPhaseEvent(@Nonnull final PlannerPhase plannerPhase,
537537
@Nonnull final Reference rootReference,
538-
@Nonnull final Deque<Task> taskStack) {
539-
super(plannerPhase, rootReference, taskStack, Location.COUNT);
538+
@Nonnull final Deque<Task> taskStack,
539+
@Nonnull final Location location) {
540+
super(plannerPhase, rootReference, taskStack, location);
540541
}
541542

542543
@Override

‎fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/debug/DebuggerWithSymbolTables.java renamed to ‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/debug/DebuggerWithSymbolTables.java

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/*
2-
* PlannerRepl.java
2+
* DebuggerWithSymbolTables.java
33
*
44
* This source file is part of the FoundationDB open source project
55
*
6-
* Copyright 2015-2020 Apple Inc. and the FoundationDB project authors
6+
* Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
77
*
88
* Licensed under the Apache License, Version 2.0 (the "License");
99
* you may not use this file except in compliance with the License.
@@ -18,17 +18,14 @@
1818
* limitations under the License.
1919
*/
2020

21-
package com.apple.foundationdb.record.query.plan.debug;
21+
package com.apple.foundationdb.record.query.plan.cascades.debug;
2222

2323
import com.apple.foundationdb.record.logging.KeyValueLogMessage;
2424
import com.apple.foundationdb.record.query.combinatorics.TopologicalSort;
2525
import com.apple.foundationdb.record.query.plan.cascades.CascadesRuleCall;
2626
import com.apple.foundationdb.record.query.plan.cascades.PlanContext;
2727
import com.apple.foundationdb.record.query.plan.cascades.Quantifier;
2828
import com.apple.foundationdb.record.query.plan.cascades.Reference;
29-
import com.apple.foundationdb.record.query.plan.cascades.debug.Debugger;
30-
import com.apple.foundationdb.record.query.plan.cascades.debug.RestartException;
31-
import com.apple.foundationdb.record.query.plan.cascades.debug.StatsMaps;
3229
import com.apple.foundationdb.record.query.plan.cascades.debug.eventprotos.PEvent;
3330
import com.apple.foundationdb.record.query.plan.cascades.expressions.RelationalExpression;
3431
import com.google.common.base.Verify;
@@ -66,6 +63,7 @@
6663
* Implementation of a debugger that maintains symbol tables for easier human consumption e.g. in test cases and/or
6764
* while debugging.
6865
*/
66+
@SuppressWarnings("PMD.SystemPrintln")
6967
public class DebuggerWithSymbolTables implements Debugger {
7068
private static final Logger logger = LoggerFactory.getLogger(DebuggerWithSymbolTables.class);
7169

@@ -172,6 +170,7 @@ void restartState() {
172170
}
173171

174172
@Override
173+
@SuppressWarnings("PMD.GuardLogStatement") // false positive
175174
public void onEvent(final Event event) {
176175
if ((queryAsString == null) || (planContext == null) || stateStack.isEmpty()) {
177176
return;
@@ -201,7 +200,7 @@ public void onEvent(final Event event) {
201200
}
202201

203202
@Nullable
204-
private static <T> T lookupInCache(final Cache<Integer, T> cache, final String identifier, final String prefix) {
203+
static <T> T lookupInCache(final Cache<Integer, T> cache, final String identifier, final String prefix) {
205204
@Nullable final Integer refId = getIdFromIdentifier(identifier, prefix);
206205
if (refId == null) {
207206
return null;
@@ -236,6 +235,7 @@ boolean isValidEntityName(@Nonnull final String identifier) {
236235
}
237236

238237
@Nullable
238+
@Override
239239
public String nameForObject(@Nonnull final Object object) {
240240
final State state = getCurrentState();
241241
if (object instanceof RelationalExpression) {
@@ -256,10 +256,12 @@ public String nameForObject(@Nonnull final Object object) {
256256
public void onDone() {
257257
if (!stateStack.isEmpty() && queryAsString != null) {
258258
final var state = Objects.requireNonNull(stateStack.peek());
259-
logger.info(KeyValueLogMessage.of("planning done",
260-
"query", Objects.requireNonNull(queryAsString).substring(0, Math.min(queryAsString.length(), 30)),
261-
"duration-in-ms", TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - state.getStartTs()),
262-
"ticks", state.getCurrentTick()));
259+
if (logger.isInfoEnabled()) {
260+
logger.info(KeyValueLogMessage.of("planning done",
261+
"query", Objects.requireNonNull(queryAsString).substring(0, Math.min(queryAsString.length(), 30)),
262+
"duration-in-ms", TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - state.getStartTs()),
263+
"ticks", state.getCurrentTick()));
264+
}
263265

264266
final var eventProtos = state.getEventProtos();
265267
if (eventProtos != null) {
@@ -278,7 +280,7 @@ public void onDone() {
278280
private static void writeEventsDelimitedToFile(final List<PEvent> eventProtos) {
279281
try {
280282
final var tempFile = File.createTempFile("events-", ".bin");
281-
try (final var fos = new FileOutputStream(tempFile)) {
283+
try (var fos = new FileOutputStream(tempFile)) {
282284
for (final var eventProto : eventProtos) {
283285
eventProto.writeDelimitedTo(fos);
284286
}
@@ -293,7 +295,7 @@ private static Iterable<PEvent> eventProtosFromFile(@Nonnull final String fileNa
293295
return () -> readEventsDelimitedFromFile(fileName);
294296
}
295297

296-
@SuppressWarnings("resource")
298+
@SuppressWarnings({"resource", "PMD.CloseResource"})
297299
@Nonnull
298300
private static Iterator<PEvent> readEventsDelimitedFromFile(@Nonnull final String fileName) {
299301
final var file = new File(fileName);
@@ -348,18 +350,23 @@ private void reset() {
348350
}
349351

350352
void logQuery() {
351-
logger.debug(KeyValueLogMessage.of("planning started", "query", queryAsString));
353+
if (logger.isDebugEnabled()) {
354+
logger.debug(KeyValueLogMessage.of("planning started", "query", queryAsString));
355+
}
352356
}
353357

354358
@Nonnull
359+
@SuppressWarnings({"PMD.AvoidPrintStackTrace", "unused", "CallToPrintStackTrace"})
355360
private <T> Optional<T> getSilently(@Nonnull final String actionName, @Nonnull final SupplierWithException<T> supplier) {
356361
try {
357362
return Optional.ofNullable(supplier.get());
358363
} catch (final RestartException rE) {
359364
throw rE;
360-
} catch (final Throwable t) {
361-
logger.warn("unable to get " + actionName + ": " + t.getMessage());
362-
t.printStackTrace();
365+
} catch (final Exception e) {
366+
if (logger.isWarnEnabled()) {
367+
logger.warn("unable to get {}: {}", actionName, e.getMessage());
368+
}
369+
e.printStackTrace();
363370
return Optional.empty();
364371
}
365372
}

‎fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/debug/State.java renamed to ‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/debug/State.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* This source file is part of the FoundationDB open source project
55
*
6-
* Copyright 2015-2020 Apple Inc. and the FoundationDB project authors
6+
* Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
77
*
88
* Licensed under the Apache License, Version 2.0 (the "License");
99
* you may not use this file except in compliance with the License.
@@ -18,17 +18,13 @@
1818
* limitations under the License.
1919
*/
2020

21-
package com.apple.foundationdb.record.query.plan.debug;
21+
package com.apple.foundationdb.record.query.plan.cascades.debug;
2222

2323
import com.apple.foundationdb.record.RecordCoreException;
2424
import com.apple.foundationdb.record.logging.KeyValueLogMessage;
2525
import com.apple.foundationdb.record.query.plan.cascades.CascadesRule;
2626
import com.apple.foundationdb.record.query.plan.cascades.Quantifier;
2727
import com.apple.foundationdb.record.query.plan.cascades.Reference;
28-
import com.apple.foundationdb.record.query.plan.cascades.debug.BrowserHelper;
29-
import com.apple.foundationdb.record.query.plan.cascades.debug.Debugger;
30-
import com.apple.foundationdb.record.query.plan.cascades.debug.Stats;
31-
import com.apple.foundationdb.record.query.plan.cascades.debug.StatsMaps;
3228
import com.apple.foundationdb.record.query.plan.cascades.debug.eventprotos.PEvent;
3329
import com.apple.foundationdb.record.query.plan.cascades.expressions.RelationalExpression;
3430
import com.apple.foundationdb.record.util.pair.Pair;
@@ -47,15 +43,15 @@
4743
import java.util.ArrayDeque;
4844
import java.util.Deque;
4945
import java.util.Iterator;
50-
import java.util.LinkedHashMap;
5146
import java.util.List;
5247
import java.util.Locale;
5348
import java.util.Map;
5449
import java.util.Objects;
5550
import java.util.concurrent.TimeUnit;
5651
import java.util.function.IntUnaryOperator;
5752

58-
class State {
53+
@SuppressWarnings("PMD.SystemPrintln")
54+
public class State {
5955
@Nonnull
6056
private static final Logger logger = LoggerFactory.getLogger(State.class);
6157

@@ -148,8 +144,8 @@ private State(@Nonnull final Map<Class<?>, Integer> classToIndexMap,
148144
@Nullable final List<Debugger.Event> events,
149145
@Nullable final List<PEvent> eventProtos,
150146
@Nullable final Iterable<PEvent> prerecordedEventProtoIterable,
151-
@Nonnull final LinkedHashMap<Class<? extends Debugger.Event>, MutableStats> eventClassStatsMap,
152-
@Nonnull final LinkedHashMap<Class<? extends CascadesRule<?>>, MutableStats> plannerRuleClassStatsMap,
147+
@Nonnull final Map<Class<? extends Debugger.Event>, MutableStats> eventClassStatsMap,
148+
@Nonnull final Map<Class<? extends CascadesRule<?>>, MutableStats> plannerRuleClassStatsMap,
153149
@Nonnull final Deque<Pair<Class<? extends Debugger.Event>, EventDurations>> eventProfilingStack,
154150
final int currentTick,
155151
final long startTs) {
@@ -327,6 +323,7 @@ public void addCurrentEvent(@Nonnull final Debugger.Event event) {
327323
break;
328324
default:
329325
updateCounts(event);
326+
break;
330327
}
331328
}
332329

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/explain/PlannerGraphVisitor.java

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import com.apple.foundationdb.record.query.plan.cascades.explain.PlannerGraph.Node;
3434
import com.apple.foundationdb.record.query.plan.cascades.explain.PlannerGraph.PartialMatchEdge;
3535
import com.apple.foundationdb.record.query.plan.cascades.expressions.RelationalExpression;
36-
import com.apple.foundationdb.record.query.plan.plans.RecordQueryPlan;
3736
import com.google.common.base.Preconditions;
3837
import com.google.common.base.Throwables;
3938
import com.google.common.base.Verify;
@@ -65,21 +64,21 @@
6564
@SuppressWarnings({"UnstableApiUsage"})
6665
public class PlannerGraphVisitor implements SimpleExpressionVisitor<PlannerGraph> {
6766

68-
public static final int EMPTY_FLAGS = 0x0000;
67+
public static final int EMPTY_FLAGS = 0x0000;
6968

7069
/**
7170
* Indicates if this property is computed for the purpose of creating an explain
7271
* of the execution plan.
7372
*/
74-
public static final int FOR_EXPLAIN = 0x0001;
73+
public static final int FOR_EXPLAIN = 0x0001;
7574

7675
/**
7776
* Indicates if {@link Reference} instances that contain exactly one variation
7877
* are rendered.
7978
*/
80-
public static final int RENDER_SINGLE_GROUPS = 0x0002;
81-
public static final int REMOVE_PLANS = 0x0004;
82-
public static final int REMOVE_LOGICAL_EXPRESSIONS = 0x0008;
79+
public static final int RENDER_SINGLE_GROUPS = 0x0002;
80+
public static final int REMOVE_FINAL_EXPRESSIONS = 0x0004;
81+
public static final int REMOVE_EXPLORATORY_EXPRESSIONS = 0x0008;
8382

8483
private final int flags;
8584

@@ -102,7 +101,7 @@ public String apply(@Nonnull final T t) {
102101
* @param flags the flags to validate
103102
*/
104103
private static boolean validateFlags(final int flags) {
105-
if ((flags & REMOVE_PLANS) != 0 && (flags & REMOVE_LOGICAL_EXPRESSIONS) != 0) {
104+
if ((flags & REMOVE_FINAL_EXPRESSIONS) != 0 && (flags & REMOVE_EXPLORATORY_EXPRESSIONS) != 0) {
106105
return false;
107106
}
108107
return true;
@@ -412,7 +411,7 @@ public static PlannerGraphVisitor forInternalShow(final boolean renderSingleGrou
412411

413412
public static PlannerGraphVisitor forInternalShow(final boolean renderSingleGroups, final boolean removePlans) {
414413
return new PlannerGraphVisitor((renderSingleGroups ? RENDER_SINGLE_GROUPS : 0) |
415-
(removePlans ? REMOVE_PLANS : 0));
414+
(removePlans ? REMOVE_FINAL_EXPRESSIONS : 0));
416415
}
417416

418417
/**
@@ -436,12 +435,12 @@ public boolean renderSingleGroups() {
436435
return (flags & RENDER_SINGLE_GROUPS) != 0;
437436
}
438437

439-
public boolean removePlansIfPossible() {
440-
return (flags & REMOVE_PLANS) != 0;
438+
public boolean removeFinalExpressionsIfPossible() {
439+
return (flags & REMOVE_FINAL_EXPRESSIONS) != 0;
441440
}
442441

443-
public boolean removeLogicalExpressions() {
444-
return (flags & REMOVE_LOGICAL_EXPRESSIONS) != 0;
442+
public boolean removeExploratoryExpressions() {
443+
return (flags & REMOVE_EXPLORATORY_EXPRESSIONS) != 0;
445444
}
446445

447446
@Nonnull
@@ -471,23 +470,26 @@ public PlannerGraph evaluateAtRef(@Nonnull final Reference ref, @Nonnull List<Pl
471470
return PlannerGraph.builder(new PlannerGraph.ReferenceHeadNode(ref)).build();
472471
}
473472

474-
if (removePlansIfPossible()) {
473+
if (removeFinalExpressionsIfPossible()) {
475474
final List<PlannerGraph> filteredMemberResults = memberResults.stream()
476475
.filter(graph -> graph.getRoot() instanceof PlannerGraph.WithExpression)
477476
.filter(graph -> {
478477
final RelationalExpression expression = ((PlannerGraph.WithExpression)graph.getRoot()).getExpression();
479-
return !(expression instanceof RecordQueryPlan);
478+
return expression != null && ref.isExploratory(expression);
480479
})
481480
.collect(Collectors.toList());
482481

483482
// if we filtered down to empty it is better to just show the physical plan, otherwise try to avoid it
484483
if (!filteredMemberResults.isEmpty()) {
485484
memberResults = filteredMemberResults;
486485
}
487-
} else if (removeLogicalExpressions()) {
486+
} else if (removeExploratoryExpressions()) {
488487
final List<PlannerGraph> filteredMemberResults = memberResults.stream()
489488
.filter(graph -> graph.getRoot() instanceof PlannerGraph.WithExpression)
490-
.filter(graph -> ((PlannerGraph.WithExpression)graph.getRoot()).getExpression() instanceof RecordQueryPlan)
489+
.filter(graph -> {
490+
final var expression = ((PlannerGraph.WithExpression)graph.getRoot()).getExpression();
491+
return expression != null && ref.isFinal(expression);
492+
})
491493
.collect(Collectors.toList());
492494

493495
// if we filtered down to empty it is better to just show the physical plan, otherwise try to avoid it

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryRecursiveUnionPlan.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ public int hashCodeWithoutChildren() {
286286
}
287287

288288
private int computeHashCodeWithoutChildren() {
289-
return Objects.hash(initialStateQuantifier, recursiveStateQuantifier);
289+
return Objects.hash(tempTableInsertAlias, tempTableInsertAlias);
290290
}
291291

292292
@Override

‎fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreConcurrentTestBase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import com.apple.foundationdb.record.query.plan.RecordQueryPlanner;
3232
import com.apple.foundationdb.record.query.plan.cascades.CascadesPlanner;
3333
import com.apple.foundationdb.record.query.plan.cascades.debug.Debugger;
34-
import com.apple.foundationdb.record.query.plan.debug.DebuggerWithSymbolTables;
34+
import com.apple.foundationdb.record.query.plan.cascades.debug.DebuggerWithSymbolTables;
3535
import com.apple.foundationdb.record.test.FDBDatabaseExtension;
3636
import com.apple.foundationdb.record.test.TestKeySpacePathManagerExtension;
3737
import com.apple.foundationdb.record.util.pair.Pair;

‎fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/rules/DecorrelateValuesRuleTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
import com.apple.foundationdb.record.query.plan.cascades.values.RecordConstructorValue;
5353
import com.apple.foundationdb.record.query.plan.cascades.values.StreamingValue;
5454
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
55-
import com.apple.foundationdb.record.query.plan.debug.DebuggerWithSymbolTables;
55+
import com.apple.foundationdb.record.query.plan.cascades.debug.DebuggerWithSymbolTables;
5656
import com.google.common.collect.ImmutableList;
5757
import com.google.common.collect.ImmutableMap;
5858
import org.junit.jupiter.api.BeforeEach;

0 commit comments

Comments
 (0)