Collected while fixing #6303 (merged as PR #6309, bdf46f9). Item 1 is a correctness defect this issue's work uncovered but deliberately did not fix; items 2 and 3 are tests that went red on #6309's CI and were triaged as not-the-PR; items 4 and 5 came out of that PR's review and were declined there as out of scope. None of them is #6303's to fix, and all of them are now written down instead of remembered.
Item 1 - INSERT ...; CREATE INDEX ... in ONE transaction leaves the record unindexed
The same failure shape as #6281, and it does not need the asynchronous API at all:
INSERT INTO V SET id = 7;
CREATE INDEX ON V (id) UNIQUE;
Run as a single sqlscript inside one transaction, this ends with the record present and no index entry for it. Measured synchronously - no async executor involved:
database.transaction(() -> database.command("sqlscript",
"INSERT INTO V SET id = 7; CREATE INDEX ON V (id) UNIQUE;").close());
// records=1 entries=0The mechanism is the one #6281 documented, arrived at from the other side. The build scans the buckets for committed records, and this record is not committed yet - it belongs to the very transaction the CREATE INDEX is running in. And it cannot fall back on staging its own index operation either, because it was saved before the index existed to stage one for. So it is in neither the scan nor the index, and the result is an index that is readable, reported healthy by CHECK DATABASE, and answers SELECT ... WHERE id = 7 with nothing.
Worth deciding what the right answer is, because there are several and they are not equally good:
- have the build also apply the uncommitted index operations of the transaction it runs in (the scan is not the only source of truth when the builder shares a transaction with a writer);
- refuse the combination outright, the way the async barrier refuses what it cannot satisfy, so the caller learns instead of being handed a quietly incomplete index;
- treat it as documented behaviour - "index before you insert, or commit between them" - which is the status quo minus the silence.
The first is the only one that makes the obvious script do the obvious thing. Whatever is chosen, the case deserves a regression test: Issue6303AsyncDispatchedDDLTest#aScriptMixingDDLAndWritesRunsOffTheWorkersAsAWhole currently documents the behaviour in a comment and deliberately does not assert an entry, so that a fix does not have to fight a test that pinned the bug.
Item 2 - FullTextBM25CompactionTest discards the boolean that says whether it compacted anything
engine/src/test/java/com/arcadedb/index/fulltext/FullTextBM25CompactionTest.java:64:
((IndexInternal) ftIndex).scheduleCompaction(); // return value discarded
compacted |= ftIndex.compact();
scheduleCompaction() is a reservation - AVAILABLE -> COMPACTION_SCHEDULED by CAS, plus a refusal when page flushing is suspended - and it returns false when it did not get it. The engine schedules compaction automatically while the test's 601 inserts run, so whenever that automatic reservation is still outstanding the test's own scheduleCompaction() loses the CAS, compact() has nothing to do, and the assertion fails with a message about the index rather than about the race:
[the full-text index should have compacted at least one bucket]
Expecting value to be true but was false
Seen once on #6309's unit-tests lane; passed on a re-run of the identical commit, and 12/12 locally in isolation. So it is a real flake with a misleading message, not a symptom.
The unchecked boolean is the same defect #6303 item 2 was about, one layer down: scheduleCompaction() answers "did the thing you asked for actually happen", and nothing reads it. The fix is to assert it (and either drain the automatic compaction first or disable it for the duration), so a lost race fails as a lost race.
Item 3 - SelectParallelIteratorTest asserts on raw wall-clock time
engine/src/test/java/com/arcadedb/query/select/SelectParallelIteratorTest.java - nine assertions of the shape:
assertThat(database.async().waitCompletion(15_000)).isTrue();
abandonedConsumerReleasesWorkersWithinStallBound failed for me at 25.1 s against that 15 s budget, on a machine running back-to-back suites, and passed 5/5 once the machine was idle. That is precisely what CLAUDE.md's #6260 rule exists for: a full-suite run shares one JVM, and a stop-the-world pause late in a 12,000-test run turns any bound with less headroom than the pause into a coin flip.
These are not all the same kind of bound, which is why the fix is not "raise 15000":
- the ones asserting that producers give up within the stall/selection timeout are tripwires between a bounded operation and an unbounded one, and should be measured with
com.arcadedb.utility.StallAwareStopwatch#assertGaveUpWithin so the JVM-wide stall inside the window is discounted;
- the ones that are merely liveness guards ("this must not hang") should say so and be sized as hang detectors.
Item 4 - four copies of one thread pool
QueryEngineManager, SparseVectorScoringPool, ParallelScanProducerPool and (since #6303) AsyncCommandPool each carry their own near-identical copy of: the ThreadPoolExecutor construction, the initialization-on-demand Holder, a record PoolStats(...) with the same six components, and a throttled-WARNING caller-runs rejection handler. SparseVectorScoringPool's own javadoc says "same shape as QueryEngineManager.PoolStats" without factoring it out.
The wording has already drifted, which is the cost showing up rather than a hypothetical:
Query parallelism pool saturated: queue full (capacity=%d, threads=%d), running task on caller thread (...)
Sparse-vector scoring pool saturated: queue full (capacity=%d, threads=%d), running task on caller thread (...)
Asynchronous command pool saturated: queue full (capacity=%d, threads=%d), running the command on the submitting thread (...)
A shared base - pool construction, PoolStats, and the rejection handler - would remove ~100 duplicated lines and, more usefully, would let the engine-concurrency skill's checklist for a new pool be enforced rather than remembered: bounded queue, caller-runs (or a documented deviation, as ParallelScanProducerPool has), throttled warning, PoolMetrics binding. Raised in review of #6309 and declined there: refactoring three working subsystems does not belong inside a concurrency bugfix.
Item 5 - asynchronously dispatched DDL works for SQL only
#6303 item 3 gave back CREATE INDEX / REBUILD INDEX sent with awaitResponse=false by routing statements that parse to a DDLStatement off the async workers. The classification is in DatabaseAsyncExecutorImpl#requiresOffWorkerExecution and covers sql and sqlscript only, because the decision has to be a parse (a keyword match would misroute ordinary writes and cost them their bucket pinning) and only SQL has one cheap enough to pay on the submitting thread.
So the equivalent statement in Cypher, Gremlin, GraphQL or the Mongo dialect still runs on a worker and is still refused there by #6281's guard. That is a refusal rather than the old hang, and the workaround is unchanged (awaitResponse=true) - but it is an asymmetry a user meets without warning. Worth doing if anyone wants Cypher CREATE INDEX fire-and-forget; the shape would be a per-language "is this DDL" hook on the query engine, so the routing stops being SQL's private knowledge.
Smaller notes, not worth items of their own
a. pageFlushQueueMaxPerDatabase still is not in Micrometer. Carried over unchanged from note (a) of #6303: it belongs to the same pass as item 2 of #6087, not to a new issue. Recording it here only so the trail from #6281 -> #6303 -> #6087 does not go cold. AsyncCommandPool is already bound as pool=async_command.
b. DatabaseContext.asyncMode is named for what it is not. It does not mean "this thread is an async worker" - #6303 note (b) removed the one place that read it that way. It means "pick a bucket per thread so concurrent writers do not compete for the same pages", and it is now set on AsyncCommandPool threads, which are deliberately not workers. The name is the only thing still suggesting the old reading; something like perThreadBucketSelection would stop the next reader having to be told.
Collected while fixing #6303 (merged as PR #6309,
bdf46f9). Item 1 is a correctness defect this issue's work uncovered but deliberately did not fix; items 2 and 3 are tests that went red on #6309's CI and were triaged as not-the-PR; items 4 and 5 came out of that PR's review and were declined there as out of scope. None of them is #6303's to fix, and all of them are now written down instead of remembered.Item 1 -
INSERT ...; CREATE INDEX ...in ONE transaction leaves the record unindexedThe same failure shape as #6281, and it does not need the asynchronous API at all:
Run as a single
sqlscriptinside one transaction, this ends with the record present and no index entry for it. Measured synchronously - no async executor involved:The mechanism is the one #6281 documented, arrived at from the other side. The build scans the buckets for committed records, and this record is not committed yet - it belongs to the very transaction the
CREATE INDEXis running in. And it cannot fall back on staging its own index operation either, because it was saved before the index existed to stage one for. So it is in neither the scan nor the index, and the result is an index that is readable, reported healthy byCHECK DATABASE, and answersSELECT ... WHERE id = 7with nothing.Worth deciding what the right answer is, because there are several and they are not equally good:
The first is the only one that makes the obvious script do the obvious thing. Whatever is chosen, the case deserves a regression test:
Issue6303AsyncDispatchedDDLTest#aScriptMixingDDLAndWritesRunsOffTheWorkersAsAWholecurrently documents the behaviour in a comment and deliberately does not assert an entry, so that a fix does not have to fight a test that pinned the bug.Item 2 -
FullTextBM25CompactionTestdiscards the boolean that says whether it compacted anythingengine/src/test/java/com/arcadedb/index/fulltext/FullTextBM25CompactionTest.java:64:scheduleCompaction()is a reservation -AVAILABLE -> COMPACTION_SCHEDULEDby CAS, plus a refusal when page flushing is suspended - and it returnsfalsewhen it did not get it. The engine schedules compaction automatically while the test's 601 inserts run, so whenever that automatic reservation is still outstanding the test's ownscheduleCompaction()loses the CAS,compact()has nothing to do, and the assertion fails with a message about the index rather than about the race:Seen once on #6309's
unit-testslane; passed on a re-run of the identical commit, and 12/12 locally in isolation. So it is a real flake with a misleading message, not a symptom.The unchecked boolean is the same defect #6303 item 2 was about, one layer down:
scheduleCompaction()answers "did the thing you asked for actually happen", and nothing reads it. The fix is to assert it (and either drain the automatic compaction first or disable it for the duration), so a lost race fails as a lost race.Item 3 -
SelectParallelIteratorTestasserts on raw wall-clock timeengine/src/test/java/com/arcadedb/query/select/SelectParallelIteratorTest.java- nine assertions of the shape:abandonedConsumerReleasesWorkersWithinStallBoundfailed for me at 25.1 s against that 15 s budget, on a machine running back-to-back suites, and passed 5/5 once the machine was idle. That is precisely what CLAUDE.md's #6260 rule exists for: a full-suite run shares one JVM, and a stop-the-world pause late in a 12,000-test run turns any bound with less headroom than the pause into a coin flip.These are not all the same kind of bound, which is why the fix is not "raise 15000":
com.arcadedb.utility.StallAwareStopwatch#assertGaveUpWithinso the JVM-wide stall inside the window is discounted;Item 4 - four copies of one thread pool
QueryEngineManager,SparseVectorScoringPool,ParallelScanProducerPooland (since #6303)AsyncCommandPooleach carry their own near-identical copy of: theThreadPoolExecutorconstruction, the initialization-on-demandHolder, arecord PoolStats(...)with the same six components, and a throttled-WARNING caller-runs rejection handler.SparseVectorScoringPool's own javadoc says "same shape asQueryEngineManager.PoolStats" without factoring it out.The wording has already drifted, which is the cost showing up rather than a hypothetical:
A shared base - pool construction,
PoolStats, and the rejection handler - would remove ~100 duplicated lines and, more usefully, would let theengine-concurrencyskill's checklist for a new pool be enforced rather than remembered: bounded queue, caller-runs (or a documented deviation, asParallelScanProducerPoolhas), throttled warning,PoolMetricsbinding. Raised in review of #6309 and declined there: refactoring three working subsystems does not belong inside a concurrency bugfix.Item 5 - asynchronously dispatched DDL works for SQL only
#6303 item 3 gave back
CREATE INDEX/REBUILD INDEXsent withawaitResponse=falseby routing statements that parse to aDDLStatementoff the async workers. The classification is inDatabaseAsyncExecutorImpl#requiresOffWorkerExecutionand coverssqlandsqlscriptonly, because the decision has to be a parse (a keyword match would misroute ordinary writes and cost them their bucket pinning) and only SQL has one cheap enough to pay on the submitting thread.So the equivalent statement in Cypher, Gremlin, GraphQL or the Mongo dialect still runs on a worker and is still refused there by #6281's guard. That is a refusal rather than the old hang, and the workaround is unchanged (
awaitResponse=true) - but it is an asymmetry a user meets without warning. Worth doing if anyone wants CypherCREATE INDEXfire-and-forget; the shape would be a per-language "is this DDL" hook on the query engine, so the routing stops being SQL's private knowledge.Smaller notes, not worth items of their own
a.
pageFlushQueueMaxPerDatabasestill is not in Micrometer. Carried over unchanged from note (a) of #6303: it belongs to the same pass as item 2 of #6087, not to a new issue. Recording it here only so the trail from #6281 -> #6303 -> #6087 does not go cold.AsyncCommandPoolis already bound aspool=async_command.b.
DatabaseContext.asyncModeis named for what it is not. It does not mean "this thread is an async worker" - #6303 note (b) removed the one place that read it that way. It means "pick a bucket per thread so concurrent writers do not compete for the same pages", and it is now set onAsyncCommandPoolthreads, which are deliberately not workers. The name is the only thing still suggesting the old reading; something likeperThreadBucketSelectionwould stop the next reader having to be told.