Skip to content

Disabling a replication domain can drop a change a replay thread is applying #908

Description

@vharseko

A domain being disabled - for an online import, a restore, or a configuration change - saves its ServerState and forgets its pending changes while a replay thread may be half way through applying one of them, so a change which reaches the backend is recorded nowhere.

Where

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java

public void disable()                                   // :3766
{
  synchronized (serviceStateLock)
  {
    state.save();                                       // :3770
    state.clearInMemory();
    disabled = true;
    disableService();                                   // joins the listener thread only
    ...
    remotePendingChanges.clear();                       // :3783
  }
}

The replay path takes serviceStateLock nowhere, and disableService() stops the broker and joins the listener thread - it does not wait for the replay threads, which are shared by every domain of the server (MultimasterReplication.replayThreads). A replay thread can therefore be between markInProgress() (:2400) and commit() with op.run() already applied to the backend when state.save() writes a watermark which excludes that change and clear() drops it.

What happens

  • The change is in the data but not in the ServerState, so the replication server sends it again when the domain is enabled back. For an Add or a Delete conflict resolution sorts it out; for a ModifyDN it can end in an entry renamed to a conflict RDN.
  • The replay thread which was applying it then finds nothing listed and logs ERR_OPERATION_NOT_FOUND_IN_PENDING (:2126, :2160).

History

Pre-existing rather than introduced by #889 / #892: before that PR disable() did not call remotePendingChanges.clear(), so the same window existed and ended in a silent second replay instead of a logged one. #892 made the outcome visible, which is how it was noticed - see the round-4 review of that PR.

What a fix looks like

The replay path has to be made visible to the domain going down: either have disable() wait for the replay threads which own a change of this domain (the ownership mark PendingChange.owned added in #892 already tells which those are), or have markInProgress()/commit() run under the same lock disable() takes, so that a change being applied either commits before the ServerState is saved or is not started at all.

Worth its own change rather than a follow-up commit on #892: it touches the ordering between the replay threads and the domain lifecycle, and it needs a test which can hold a replay thread inside op.run() while the domain is disabled.

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