From 6dc8f80457d5e4f3182fcee3adeda01928ae68f3 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Wed, 16 Sep 2026 04:55:22 +0000
Subject: [PATCH] [#953] Refuse a server-error-result-code which does not report a failure (#980)
---
opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java | 74 ++++++++++++++++++++++++++++++-------
1 files changed, 60 insertions(+), 14 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java
index 0b87409..f8c6fcc 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java
@@ -459,9 +459,10 @@
new AtomicLong(UNREPLAYED_CHANGE_ALERT_NEVER_SENT);
/**
* The result codes conflict resolution knows how to solve. The result code the server
- * puts on an internal error is configurable and is not validated as a result code, so
- * it could be set to one of these: it must never take a change away from
- * {@code solveNamingConflict()}, which is the only thing which can solve them.
+ * puts on an internal error is configurable, and every one of these reports a failure -
+ * which is all the configuration asks of it - so it can be set to one of them: it must
+ * never take a change away from {@code solveNamingConflict()}, which is the only thing
+ * which can solve them.
*/
private static final Set<ResultCode> CONFLICT_RESULT_CODES = Collections.unmodifiableSet(
newHashSet(
@@ -470,6 +471,23 @@
// solveNamingConflict(ModifyDNOperation) solves these two as well
ResultCode.UNWILLING_TO_PERFORM, ResultCode.OBJECTCLASS_VIOLATION));
+ /**
+ * The attachment which says that conflict resolution turned this operation into a
+ * no-op, so that {@link #replay} reads that decision rather than the result code which
+ * reports it.
+ * <p>
+ * The code conflict resolution reports for a no-op is {@code NO_OPERATION}, and the
+ * code this server puts on an internal error is a configuration knob: while nothing
+ * validated it, the two could be the same code, and every change an internal error kept
+ * out of the backend was then read as a change conflict resolution had found already
+ * applied and recorded in the ServerState - the silent divergence of issue #889, one
+ * branch earlier (issue #953). The configuration refuses a code which does not report a
+ * failure now, so they can not be the same code anymore; the decision travels on the
+ * operation all the same, so that what the replay acts on is what conflict resolution
+ * decided rather than a value an administrator owns.
+ */
+ private static final String CONFLICT_RESOLUTION_NO_OP = "replicationConflictResolutionNoOp";
+
private final PersistentServerState state;
private volatile boolean generationIdSavedStatus;
@@ -1915,8 +1933,7 @@
}
if (replayedEntryDN != null)
{
- return new SynchronizationProviderResult.StopProcessing(
- ResultCode.NO_OPERATION, null);
+ return conflictResolutionFoundNothingToDo(addOperation);
}
/* The parent entry may have been renamed here since the change was done
@@ -2112,8 +2129,7 @@
modifyDNOperation.getOriginalEntry());
if (hist.addedOrRenamedAfter(ctx.getCSN()))
{
- return new SynchronizationProviderResult.StopProcessing(
- ResultCode.NO_OPERATION, null);
+ return conflictResolutionFoundNothingToDo(modifyDNOperation);
}
}
else
@@ -2168,8 +2184,7 @@
{
// Every modifications filtered in this operation: the operation
// becomes a no-op
- return new SynchronizationProviderResult.StopProcessing(
- ResultCode.NO_OPERATION, null);
+ return conflictResolutionFoundNothingToDo(modifyOperation);
}
}
else
@@ -2926,7 +2941,7 @@
if (result != ResultCode.SUCCESS)
{
- if (result == ResultCode.NO_OPERATION)
+ if (isConflictResolutionNoOp(op))
{
// Pre-operation conflict resolution detected that the operation
// was a no-op. For example, an add which has already been
@@ -3450,6 +3465,37 @@
}
/**
+ * Stops an operation conflict resolution found nothing left to do for, and marks it so
+ * that the replay reads that decision off the operation rather than off the result code
+ * this answer carries.
+ *
+ * @param op the operation conflict resolution turned into a no-op
+ * @return the answer which stops the operation
+ */
+ private static SynchronizationProviderResult conflictResolutionFoundNothingToDo(PluginOperation op)
+ {
+ op.setAttachment(CONFLICT_RESOLUTION_NO_OP, Boolean.TRUE);
+ return new SynchronizationProviderResult.StopProcessing(ResultCode.NO_OPERATION, null);
+ }
+
+ /**
+ * Returns whether conflict resolution turned the replayed operation into a no-op, which
+ * says that the change it carries is in the data and can be recorded as replayed.
+ * <p>
+ * Only {@link #conflictResolutionFoundNothingToDo} answers {@code true} here. The
+ * result code that answer carries says the same thing, but it is a code the
+ * configuration can name as well - see {@link #CONFLICT_RESOLUTION_NO_OP} - and a
+ * change which failed must never be read as one which was already applied.
+ *
+ * @param op the operation which was replayed
+ * @return {@code true} if conflict resolution found nothing left to do for the change
+ */
+ private static boolean isConflictResolutionNoOp(Operation op)
+ {
+ return Boolean.TRUE.equals(op.getAttachment(CONFLICT_RESOLUTION_NO_OP));
+ }
+
+ /**
* Returns whether the provided result code reports a failure of this server rather
* than a change which can not be applied: the backend being offline or rebuilt
* (OPENDJ-49), or the storage failing to serve the operation.
@@ -3465,10 +3511,10 @@
static boolean isServerFailure(ResultCode result, ResultCode serverErrorResultCode)
{
/*
- * The result code the server puts on an internal error is configurable and is not
- * validated as a result code, so it may well be one conflict resolution knows how to
- * solve: such a setting must not take a change away from solveNamingConflict(), which
- * is the only thing which can solve them. A change it could not solve either is a
+ * The result code the server puts on an internal error is configurable and only has
+ * to report a failure, so it may well be one conflict resolution knows how to solve:
+ * such a setting must not take a change away from solveNamingConflict(), which is
+ * the only thing which can solve them. A change it could not solve either is a
* failure of the server all the same, which replay() acts on once conflict resolution
* has reported it.
*/
--
Gitblit v1.10.0