A failed entryUUID search reads as a deleted entry, and conflict resolution records the change as replayed

[vharseko]

solveNamingConflict() decides an entry is gone by searching for its entryUUID and getting nothing back. The search returning nothing and the search failing are the same answer, and the answer is recorded in the ServerState as a change which is in the data.

The conflation

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

private static SearchResultEntry getFirstResult(InternalSearchOperation search)   // :3279
{
  if (search.getResultCode() == ResultCode.SUCCESS)
  {
    final LinkedList<SearchResultEntry> results = search.getSearchEntries();
    if (!results.isEmpty())
    {
      return results.getFirst();
    }
  }
  return null;                       // no entry, or the search never ran
}

findEntryDN() (:3299) hands that null on, and every caller in conflict resolution reads it as "the entry has been deleted":

overload	branch	reads null as
ModifyOperation :3350	NO_SUCH_OBJECT	"This entry does not exist anymore. It has probably been deleted" -> NOTHING_TO_DO (:3364)
ModifyOperation :3369	NOT_ALLOWED_ON_RDN	"The entry does not exist anymore." -> NOTHING_TO_DO (:3379)
DeleteOperation :3431	NO_SUCH_OBJECT	"The entry has already been deleted" -> NOTHING_TO_DO (:3441)
ModifyDNOperation :3507	any	"The entry targeted by the Modify DN is not in the database anymore" -> NOTHING_TO_DO (:3545)

(solveNamingConflict(AddOperation) is not in this list: its findEntryDN() returning null renames the entry as conflicting rather than answering NOTHING_TO_DO.)

Where it lands

switch (resolution)
{
case NOTHING_TO_DO:                     // :2613
  // the update became a dummy update and the result
  // of the conflict resolution phase is to do nothing.
  // however we still need to push this change to the serverState
  replayDone = true;
  recordChangeResolved(csn);            // :2618
  break;

case FAILED:                            // :2621
  if (serverErrorResultCode.equals(result))   // :2622
  {
    ...

NOTHING_TO_DO commits the CSN unconditionally. Two lines below, FAILED carries the guard #892 added for exactly this shape of mistake - a result which looks like a verdict on the change but is really the server failing. NOTHING_TO_DO never got one.

So a change which was never applied is recorded as replayed, the replication server never sends it again because this replica reports itself past that CSN, and no alert is raised. That is the #889 failure mode, through a branch #892 did not harden.

Two ways in

With the default configuration. server-error-result-code is 80, a replayed Modify comes back NO_SUCH_OBJECT because the entry really was renamed elsewhere, and isServerFailure(32, 80) is false, so conflict resolution gets it as intended. Its findEntryDN() search then hits a moment where the backend does not serve it - and the entry, which exists under its new DN, is written off as deleted. No unusual configuration at all: only a search which fails while the operation before it did not.

With server-error-result-code set to one of the conflict codes. Set it to 32. A replayed Modify hits a failing backend, so the server puts NO_SUCH_OBJECT on it. isServerFailure(32, 32) is false by design (:2873, CONFLICT_RESULT_CODES at :423) - that carve-out is what #892 added so a conflict is not taken away from solveNamingConflict() - and the change is handed to conflict resolution. Its search runs over the same failing backend, returns nothing, and the storage failure is recorded as a resolved conflict. Here the carve-out which fixed #889 is what opens the door.

Not the other two

• A change which never applied is recorded as replayed when server-error-result-code is set to NO_OPERATION or SUCCESS #953 is SUCCESS, NO_OPERATION and BUSY being read before either guard is consulted.
• NPE in solveNamingConflict(ModifyDNOperation) when both the moved entry and its new parent are gone #955 is the NPE in solveNamingConflict(ModifyDNOperation).

This one is about what conflict resolution concludes once it is correctly given the change, and about the branch which records that conclusion.

What is worth doing

Two independent halves, and the first is worth having on its own:

1. getFirstResult() should not answer the same thing for a search which failed and a search which found nothing - and findEntryDN() should not turn a failed search into "the entry is gone". A search which did not run is not evidence about the data.
2. case NOTHING_TO_DO should not commit the CSN while the result code is the configured server-error-result-code, the same carve-out case FAILED has at :2622. That half only covers the second way in; the first needs the first half.

Found in a review of the #939 branch. Unrelated to it - #939 is a test for isServerFailure() and changes no behaviour.