Skip to content

HA: OpenCypherQueryEngine commits on the inner LocalDatabase, bypassing Raft replication (same shape as #5492) #5655

Description

@robfrank

Summary

OpenCypherQueryEngine commits on the inner LocalDatabase rather than the Raft wrapper, which is the same defect #5492 fixed for SQLQueryEngine in #5652. On an HA leader this applies pages locally and never proposes them to Raft.

Found while reviewing #5652; filed separately rather than folded in, since it needs its own reproduction and test.

The shape

OpenCypherQueryEngine holds private final DatabaseInternal database (line 83), assigned in the constructor (line 85). On an HA leader that is the inner LocalDatabase: RaftReplicatedDatabase.command() delegates to proxied.command(...), and RaftReplicatedDatabase.getQueryEngine() (line 1197) delegates to proxied.getQueryEngine(), so the engine is built with the inner instance.

The explicit-transaction path then commits on that field directly:

// OpenCypherQueryEngine.java:685-693
database.begin(level);        // or database.begin()
...
case COMMIT:
  if (!database.isTransactionActive())
    throw new CommandExecutionException("No active transaction to COMMIT (issue a START TRANSACTION first)");
  database.commit();          // <-- LocalDatabase.commit() on an HA leader: applies pages, replicates nothing

context.setDatabase(database) (line 738) passes the same inner instance to statement execution, so any Cypher statement that commits mid-execution has the same exposure.

Why this matters

LocalDatabase.commit() writes pages locally; RaftReplicatedDatabase.commit() proposes to Raft and then writes. Committing on the inner instance succeeds and replicates nothing. Followers then trail by exactly those page versions, and the next replicated entry touching one of those pages fails its version check with WALVersionGapException - database marked diverged, snapshot resync, and the entry after it breaks the same way.

That is the failure #5492 produced through TRUNCATE TYPE: 24002 page-version gaps and 357 resync cycles per follower, with one node ending short by scan and its materialized view empty. Nothing throws at the point of the mistake, which is what makes this class of bug hard to attribute.

Not yet verified

I have not reproduced this. The code shape matches #5492 exactly, but two things need checking before treating it as confirmed:

  1. Whether a Cypher explicit transaction is reachable in a way that spans the wrapper on an HA leader - over HTTP each command is a separate request, so the practical exposure depends on how START TRANSACTION / COMMIT bind to a session.
  2. Whether autocommit Cypher writes are already covered by the wrapper's own commit finalization, in which case only the explicit-transaction path is exposed.

Suggested fix

Same resolution as #5652: resolve getWrappedDatabaseInstance() for anything that can commit. Off HA it returns the instance itself, so it is a no-op there. SQLQueryEngine.executionDatabase() carries the reasoning and the carve-out for idempotent read paths.

Test

A Cypher analogue of Issue5492TruncateBatchNotReplicatedIT: 2-node cluster, explicit START TRANSACTION / write / COMMIT on the leader, assert ArcadeStateMachine.TEST_WAL_GAP_COUNTER stays 0 and the follower converges. Assert the gap counter before querying the follower - a resync reinstalls the follower's database and the later query then fails with DatabaseIsClosed, which reads like infrastructure noise rather than the divergence that caused it.

Related: #5492, #5652. The rule is recorded in ha-raft/CLAUDE.md under "Anything that commits must hold the WRAPPED database instance, not the inner one".

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions