Skip to content

Replication: a change parked as a dependency is left owned by a thread which never comes back to it #954

Description

@vharseko

A change which waits for another one is parked: RemotePendingChanges.checkDependencies() puts it in dependentChanges, and it stays owned by the replay thread which parked it while that thread goes on to the changes which follow. getNextUpdate() is what hands it out again - to whichever replay thread clears the change it was waiting for, which takes it over - so the ownership of a parked change is deliberately held by a thread which is not replaying it right now (opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/RemotePendingChanges.java).

That holds for as long as the parking thread comes back to the pool and keeps taking changes. It does not hold when its replay() is unwound while it still holds one: the give-back on the way out added for #922 leaves parked changes alone on purpose - releasing one would let a new delivery be taken over while getNextUpdate() is still holding the same PendingChange to hand out, which is the double replay the ownership exists to prevent - so the parked change is left owned by a thread which will never come back to it. Two roads do that:

Nothing else asks for the change: putRemoteUpdate() refuses every redelivery of a change a replay thread owns, and a markInProgress() which returns false has the replay thread continue without reaching getNextUpdate(). It is handed out again only when some other change is replayed on this domain and that replay loop reaches its own getNextUpdate(). On a domain which then goes quiet the parked change is the tail: this replica's ServerState - and every change behind it, from every master - stops advancing until something else arrives.

Scope

The give-back on the way out has to be able to hand a parked change back as well, which means taking it out of dependentChanges in the same step, so that only one road can ever hand it out:

  • RemotePendingChanges: a give-back which releases every change the calling thread owns and unparks the ones which are parked, under both locks;
  • LDAPReplicationDomain.replay(): use it on the way out, and ask for what it released to be delivered again - a session restart is what brings a change back.

A test is the hard part: parking a change needs two changes which depend on one another to be in flight at the same time, and the thread holding the parked one has to be the one which then meets the error.

How narrow this is

Everything has to line up: a thread parks a change, the same replay() call then unwinds, and no further change reaches the domain afterwards. It is also not something #922 introduced - a parked change was only ever revived by another replay loop - which is why it is here rather than in that fix.

Found while reviewing the fix for #922 and #923.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions