Skip to content

Commit c0fcb7c

Browse files
authored
Run expensive Traversal verification sanity check less often (#3442)
A sanity check to validate the traversal integrity was recently added in #3435. That check alone seems to almost double the total time it takes to run the `core-tests` during PRB from around 15 minutes to around 30 minutes. It's a useful check, but it is probably running too often. We should be able to get away with only running it occaisionally and then only enabling it more stringently if the need arises.
1 parent be3cca6 commit c0fcb7c

File tree

1 file changed

+19
-5
lines changed
  • fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades

1 file changed

+19
-5
lines changed

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -437,11 +437,22 @@ private void planPartial(@Nonnull final Supplier<Reference> referenceSupplier,
437437
Debugger.withDebugger(debugger -> debugger.onEvent(nextTask.toTaskEvent(Location.BEGIN)));
438438
try {
439439
nextTask.execute();
440-
Debugger.sanityCheck(() ->
441-
// This is a fairly expensive check, but it makes sure that the traversal
442-
// is up to date with the query DAG. This is used during memoization, so
443-
// it's important that it has an accurate view of the state of the world.
444-
traversal.verifyIntegrity());
440+
Debugger.sanityCheck(() -> {
441+
if (nextTask instanceof InitiatePlannerPhase) {
442+
//
443+
// Validate that the memo structure and the query graph are aligned.
444+
// This is an expensive check, but if it finds anything, then it may
445+
// be useful to enable this check so that it runs after every task
446+
// to root out the underlying issue.
447+
//
448+
// Once we have sanity check levels, we should consider enabling
449+
// this check all the time at certain levels.
450+
// See: https://github.com/FoundationDB/fdb-record-layer/issues/3445
451+
//
452+
traversal.verifyIntegrity();
453+
}
454+
});
455+
445456
} finally {
446457
Debugger.withDebugger(debugger -> debugger.onEvent(nextTask.toTaskEvent(Location.END)));
447458
}
@@ -472,6 +483,9 @@ private void planPartial(@Nonnull final Supplier<Reference> referenceSupplier,
472483
pushInitialTasks();
473484
}
474485
}
486+
487+
// Also validate memo structure integrity at the end of planning
488+
Debugger.sanityCheck(() -> traversal.verifyIntegrity());
475489
}
476490

477491
private void pushInitialTasks() {

0 commit comments

Comments
 (0)