[#1040] Leave the session to a total update when a configuration change asks for a restart

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

@@ -153,12 +153,26 @@ public boolean isConfigurationAddAcceptable(
   {
     return true;
   }
-  /** {@inheritDoc} */
+  /**
+   * {@inheritDoc}
+   * <p>
+   * Refused while a total update runs, as a change of the domain entry is: the attributes
+   * this entry carries are published as the session of the domain comes up, so applying
+   * them restarts that session, and an import into this replica reads its entries over
+   * it. Through the server configuration such a restart would wait for the listener
+   * thread the import runs on, which waits in turn for the lock of the configuration the
+   * change holds, to enable the backend back once the stream it was reading ends.
+   */
   @Override
   public boolean isConfigurationChangeAcceptable(
       ExternalChangelogDomainCfg configuration,
       List<LocalizableMessage> unacceptableReasons)
   {
+    if (domain.ieRunning())
+    {
+      unacceptableReasons.add(NOTE_ERR_CANNOT_CHANGE_CONFIG_DURING_TOTAL_UPDATE.get());
+      return false;
+    }
     return true;
   }
 

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

@@ -5531,8 +5531,9 @@ public ConfigChangeResult applyConfigurationChange(
         changeConfig(configuration);
 
         // Read assured + fractional configuration and each time reconnect if needed. A
-        // domain which owns its session gets none of those reconnections.
-        final boolean allowReconnection = !ownsItsSession();
+        // session which has an owner - the domain itself, or a total update into this
+        // replica - gets none of those reconnections.
+        final boolean allowReconnection = !sessionHasAnOwner();
         readAssuredConfig(configuration, allowReconnection);
         readFractionalConfig(configuration, allowReconnection);
         solveConflictFlag = isSolveConflict(configuration);
@@ -5580,9 +5581,10 @@ private boolean ownsItsSession()
   }
 
   /**
-   * Whether the session of this domain has an owner other than the replay thread which
-   * would restart it after a failed replay: the domain itself, when it is shutting down or
-   * disabled ({@link #ownsItsSession()}), or a total update into this replica.
+   * Whether the session of this domain has an owner other than the thread which would
+   * restart it - a replay thread after a failed replay, or a configuration change for what
+   * it carries: the domain itself, when it is shutting down or disabled
+   * ({@link #ownsItsSession()}), or a total update into this replica.
    * <p>
    * The total update owns the session from the moment it is asked for, not from the
    * moment its entries stream: the {@code InitializeTargetMsg} which answers the request
@@ -5596,6 +5598,15 @@ private boolean ownsItsSession()
    * Listed, it holds the ServerState back as well: a commit moves the state no further than
    * the oldest uncommitted change, so the state in memory, and the one persisted from it,
    * stop at the change until that restart.
+   * <p>
+   * A configuration change is refused while a total update runs - by the listener of the
+   * domain entry and by the one of its external changelog entry - so what reaches the
+   * domain all the same is a change accepted before the total update was asked for. The
+   * restart it asks for is refused on this predicate too, and the session the import
+   * starts when it ends reads the configuration stored meanwhile. Made through the server
+   * configuration, that restart would not end: it waits for the listener thread, which is
+   * the import, and the import waits for the lock of the configuration the change holds,
+   * to enable the backend back once the stream it was reading ends.
    */
   private boolean sessionHasAnOwner()
   {
@@ -5607,21 +5618,30 @@ protected void restartService()
   {
     synchronized (serviceStateLock)
     {
-      if (ownsItsSession())
+      if (sessionHasAnOwner())
       {
         /*
-         * The domain is going away or is being imported into: a restart here would bring
-         * a session, and the listener thread which goes with it, back up on a domain
-         * whose ServerState is gone from memory. The session started when the domain is
-         * enabled again reads the configuration this restart was asked for.
+         * The domain is going away or is disabled: a restart here would bring a session,
+         * and the listener thread which goes with it, back up on a domain whose
+         * ServerState is gone from memory. The session started when the domain is enabled
+         * again reads the configuration this restart was asked for.
+         *
+         * Or a total update into this replica is reading the session: the import streams
+         * over it, on the listener thread a restart would stop and wait for. Stopped, the
+         * broker ends the stream on the entries which had arrived; waited for, the listener
+         * thread ends the import and enables the backend back through the server
+         * configuration - whose lock a change made through it holds while it waits. The
+         * import starts the next session itself when it ends, from the state it loaded,
+         * and that session reads the configuration this restart was asked for.
          *
          * Recorded rather than passed over in silence: the configuration a restart was
          * asked for is stored, and it is the session which is not brought up on it, so a
          * change which reports plain success would have the administrator believe the
          * domain is running on it already. A domain disabled for a total update comes up
-         * on it when the total update ends; one which stays disabled - enable() gives up
-         * when the data state it reads cannot be loaded, and nothing calls it again -
-         * never does, and that is what the administrator is told to act on.
+         * on it when the total update ends, and so does one being imported into; one which
+         * stays disabled - enable() gives up when the data state it reads cannot be
+         * loaded, and nothing calls it again - never does, and that is what the
+         * administrator is told to act on.
          */
         onSessionRestartSuppressed();
         return;
@@ -5672,7 +5692,13 @@ private LocalizableMessage configChangeFailed(ReplicationDomainCfg domCfg, Excep
   public boolean isConfigurationChangeAcceptable(
          ReplicationDomainCfg configuration, List<LocalizableMessage> unacceptableReasons)
   {
-    // Check that a import/export is not in progress
+    /*
+     * Check that a import/export is not in progress. The listener of the external
+     * changelog entry of this domain refuses its change for the same reason: what either
+     * change restarts the session for, an import into this replica is reading over that
+     * session. One which starts between this check and the change being applied meets
+     * the restart guard instead (see sessionHasAnOwner()).
+     */
     if (ieRunning())
     {
       unacceptableReasons.add(

opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java

@@ -3487,8 +3487,9 @@ public void changeConfig(Set<String> includeAttributes,
    * back between the stop and the start, and both halves are counted by the session
    * generation. A subclass may leave it alone: a domain which is shutting
    * down, or which was disabled for a total update, owns its session and is not given one
-   * back by a configuration change. One which does reports it through
-   * {@link #onSessionRestartSuppressed()}.
+   * back by a configuration change, and a total update into this replica reads its
+   * entries over the session and starts the next one itself. One which does reports it
+   * through {@link #onSessionRestartSuppressed()}.
    */
   protected void restartService()
   {
@@ -3505,9 +3506,10 @@ protected void restartService()
    * <p>
    * The configuration is stored either way, and the session started next reads it - so
    * this says that the change is not live yet rather than that it was lost. A domain
-   * which restarts its session for every change never reaches this; one which owns its
-   * session while it is shutting down or disabled for a total update overrides it to tell
-   * the administrator what is waiting for that session.
+   * which restarts its session for every change never reaches this; one whose session has
+   * an owner - itself while it is shutting down or disabled for a total update, or a total
+   * update into this replica reading it - overrides it to tell the administrator what is
+   * waiting for that session.
    */
   protected void onSessionRestartSuppressed()
   {
@@ -4002,9 +4004,10 @@ else if (needReconnection)
        * configuration is: the assured timeout is the one property a session does not have
        * to be restarted for, so a change carrying it alone - reported as applied and then
        * dropped, before - is applied here. A caller which does not allow the reconnection
-       * has no session running assured replication either: the domain is being built, is
-       * shutting down, or is disabled for the length of a total update, and the session
-       * its enable() starts reads what is stored here.
+       * has no session to negotiate it over: the domain is being built, is shutting down,
+       * or is disabled for the length of a total update - or a total update into this
+       * replica is reading the session, which it must not stop - and the session started
+       * next, by enable() or by the import when it ends, reads what is stored here.
        */
       assuredConfig = config;
       if (needRestart)

opendj-server-legacy/src/messages/org/opends/messages/replication.properties

@@ -683,9 +683,10 @@ ERR_REPLICATION_DOMAIN_CONFIG_CHANGE_FAILED_326=Could not apply a configuration
  replication domain on "%s": %s
 NOTE_REPLICATION_DOMAIN_SESSION_NOT_RESTARTED_327=The configuration change was applied to the \
  replication domain on "%s", but the session to the replication server was not restarted for it: \
- the domain is shutting down, or it is disabled for the length of a total update. The change is \
- stored and takes effect when the session is started again, which a domain left disabled by a \
- failed import or restore never does
+ the domain is shutting down, it is disabled for the length of a total update, or a total update \
+ into this replica is reading that session. The change is stored and takes effect when the \
+ session is started again - a total update starts it again when it ends, a domain left disabled \
+ by a failed import or restore never does
 ERR_REPLAY_ENTRYUUID_SEARCH_FAILED_322=Could not read the data to check change %s for a conflict \
  in domain "%s": the search of the entry with entryUUID %s did not run (%s). The change is not \
  applied on what a search which read nothing seemed to say about the data, and is not recorded \