Skip to content

A ServerState update landing during a save is marked saved and never written to disk #916

Description

@vharseko

Symptom

SchemaReplicationTest.pushSchemaFilesChange fails intermittently in CI. Seen on build-maven (ubuntu-latest, 21) of run 33668788856 (PR #893, whose change touches only the JDBC backend); the other eleven jobs of the same run, running the same 32152 tests, passed.

[ERROR] SchemaReplicationTest.pushSchemaFilesChange:233
  The Schema persistentState (CSN:000001a063a153b7000100000003) has not been saved to
  .../SchemaReplicationTest/package-instance/config/schema/99-user.ldif : dn: cn=schema
  ...
  ds-sync-state: 000001a063a15094000100000002
  ds-sync-state: 000001a063a15226000200000001
  modifyTimestamp: 20260902193831Z
  expected [true] but found [false]

The test published a schema change, received the ModifyMsg back from the broker, and then waited 10 s for the CSN of that change to reach 99-user.ldif (SchemaReplicationTest.java:229-241). It never did: the file still carried serverId 1, seq 2 while serverId 1, seq 3 was awaited, and its modifyTimestamp — 19:38:31 — predates the start of the test method (19:38:32.3). Over the whole 10 s window the file was not rewritten once, although the checkpointer ticks every second.

What the run rules out

  • The checkpointer thread was alive and idle in its normal wait — the dump taken on failure shows "Replica DS(1) state checkpointer for domain "cn=schema"" daemon Id=94 TIMED_WAITING on LDAPReplicationDomain$ServerStateFlush.
  • The write was not attempted and rejected: runUpdateStateEntry() logs DEBUG_ERROR_UPDATING_RUV on any result code other than success, and the server error log is silent between 19:38:32 and the timer giving up at 19:38:42.
  • The state did reach memory: ReplicationDomain.publish() calls state.update(csn) synchronously (ReplicationDomain.java:3659-3667), and the test had already received the message that publish sent.
  • A REPLACE of ds-sync-state does rewrite the file synchronously inside the modify — SchemaBackend puts a non-schema attribute into extraAttributes and adds FILE_USER_SCHEMA_ELEMENTS to modifiedSchemaFiles (SchemaBackend.java:577-592).

So the modify that would have written the awaited CSN was never issued.

The race

PersistentServerState.save() decides whether to write from a flag that the write itself clears:

// PersistentServerState.java:108-114
public void save()
{
  if (!state.isSaved())
  {
    state.setSaved(updateStateEntry());
  }
}

updateStateEntry() serialises the state once, at the top of runUpdateStateEntry()new LDAPAttribute(REPLICATION_STATE, state.toASN1ArrayList()) — and only then runs the modify, which rewrites 99-user.ldif and takes hundreds of milliseconds. ServerState.update() sets saved = false unconditionally (ServerState.java:78-85).

A publish() landing after the snapshot is taken and before setSaved(true) is therefore lost: its CSN is not in the attribute being written, and the saved = false it set is overwritten by the setSaved(true) of the write that did not carry it. Nothing sets the flag again, so the state is not written until the next meaningful update on that domain.

On a busy domain the next update arrives shortly and the loss heals itself, which is why this is not seen elsewhere. cn=schema is the domain where changes are rare, so a lost flag can leave the persisted state stale indefinitely — and that is exactly what the test observes: one save at 19:38:31, and then silence.

Impact beyond the test

ds-sync-state on disk can lag the replica's real state by an unbounded amount, until some later change happens to arrive. After a crash or restart the domain resumes from that stale point, and for the schema domain the window can be the whole life of the server. It is the safe direction (changes are replayed rather than skipped), but it defeats the checkpointing the flush thread exists to provide, and it makes any test — or operator — that reads the persisted state see something arbitrarily old.

Suggested fix

Clear the flag before the snapshot rather than after the write, so that an update racing with the write leaves the flag set for the next tick:

public void save()
{
  if (!state.isSaved())
  {
    state.setSaved(true);          // an update() from here on clears it again
    if (!updateStateEntry())
    {
      state.setSaved(false);
    }
  }
}

The cost is at most one redundant write per race. A version counter compared across the write would achieve the same without that redundancy.

A regression test can drive it directly: hold the modify (or simply publish a change while a save is in flight) and assert that the following tick writes the newer CSN.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugconcurrencyThread-safety / race-condition bugsjavaPull requests that update java codereplicationtestsTest suites: fixing, enabling, un-disabling

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions