Ticket 48118 - At startup, changelog can be erronously rebuilt after a normal shutdown

    Problem: There are two problems that can lead to inconsistent database and changelog maxruv:
    1] the database ruv is written periodically in th ehouskeeping thread and at shutdown. It
       relies on teh ruv_dirty flag, but due to a race condition this can be reset befor writing
       the ruv
    2] the changelog max ruv is updated whenever an operation is commutted, but in case of internal
       operations inside the txn for a client operation, if the operation is aborted the cl maxruv
       is not reset. Since it is only written at shutdown this normally is no problem, but if the
       aborted operation is the last before shutdown or is aborted by shutdown the cl ruv is incorrect

    Fix: the fix is in two parts:
    1] remove the use of the dirty flag, ensure that the ruv is always written. The overhead for writing
       a database ruv that has not changed is minimal
    2] when writing the changelog maxruv check if the macsns it contains are really present in the
       changelog. If not the maxruv is not written, it will be reconstructed at the next startup

    Reviewed by: William,Thierry - Thanks

ldap/servers/plugins/replication/cl5_api.c

@@ -250,6 +250,8 @@ static void _cl5ReadBerval(struct berval *bv, char **buff);
 static void _cl5WriteBerval(struct berval *bv, char **buff);
 static int _cl5ReadBervals(struct berval ***bv, char **buff, unsigned int size);
 static int _cl5WriteBervals(struct berval **bv, char **buff, u_int32_t *size);
+static int64_t _cl5CheckMaxRUV(CL5DBFile *file, RUV *maxruv);
+static int64_t _cl5CheckCSNinCL(const ruv_enum_data *element, void *arg);
 
 /* replay iteration */
 #ifdef FOR_DEBUGGING
@@ -2716,6 +2718,36 @@ _cl5WriteBervals(struct berval **bv, char **buff, u_int32_t *size)
     return CL5_SUCCESS;
 }
 
+static int64_t
+_cl5CheckCSNinCL(const ruv_enum_data *element, void *arg)
+{
+    CL5DBFile *file = (CL5DBFile *)arg;
+    int rc = 0;
+
+    DBT key = {0}, data = {0};
+    char csnStr[CSN_STRSIZE];
+
+    /* construct the key */
+    key.data = csn_as_string(element->csn, PR_FALSE, csnStr);
+    key.size = CSN_STRSIZE;
+
+    data.flags = DB_DBT_MALLOC;
+
+    rc = file->db->get(file->db, NULL /*txn*/, &key, &data, 0);
+
+    slapi_ch_free(&(data.data));
+    return rc;
+}
+
+static int64_t
+_cl5CheckMaxRUV(CL5DBFile *file, RUV *maxruv)
+{
+    int rc = 0;
+
+    rc = ruv_enumerate_elements(maxruv, _cl5CheckCSNinCL, (void *)file);
+
+    return rc;
+}
 /* upgrade from db33 to db41
  * 1. Run recovery on the database environment using the DB_ENV->open method
  * 2. Remove any Berkeley DB environment using the DB_ENV->remove method
@@ -4010,6 +4042,13 @@ _cl5WriteRUV(CL5DBFile *file, PRBool purge)
         rc = ruv_to_bervals(file->maxRUV, &vals);
     }
 
+    if (!purge && _cl5CheckMaxRUV(file, file->maxRUV)) {
+        slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name_cl,
+                      "_cl5WriteRUV - changelog maxRUV not found in changelog for file %s\n",
+                      file->name);
+        return CL5_DB_ERROR;
+    }
+
     key.size = CSN_STRSIZE;
 
     rc = _cl5WriteBervals(vals, &buff, &data.size);

ldap/servers/plugins/replication/repl5.h

@@ -725,7 +725,6 @@ Object *replica_get_for_backend(const char *be_name);
 void replica_set_purge_delay(Replica *r, uint32_t purge_delay);
 void replica_set_tombstone_reap_interval(Replica *r, long interval);
 void replica_update_ruv_consumer(Replica *r, RUV *supplier_ruv);
-void replica_set_ruv_dirty(Replica *r);
 Slapi_Entry *get_in_memory_ruv(Slapi_DN *suffix_sdn);
 int replica_write_ruv(Replica *r);
 char *replica_get_dn(Replica *r);

ldap/servers/plugins/replication/repl5_replica.c

@@ -41,7 +41,6 @@ struct replica
     ReplicaType repl_type;             /* is this replica read-only ? */
     ReplicaId repl_rid;                /* replicaID */
     Object *repl_ruv;                  /* replica update vector */
-    PRBool repl_ruv_dirty;             /* Dirty flag for ruv */
     CSNPL *min_csn_pl;                 /* Pending list for minimal CSN */
     void *csn_pl_reg_id;               /* registration assignment for csn callbacks */
     unsigned long repl_state_flags;    /* state flags */
@@ -788,7 +787,6 @@ replica_set_ruv(Replica *r, RUV *ruv)
     }
 
     r->repl_ruv = object_new((void *)ruv, (FNFree)ruv_destroy);
-    r->repl_ruv_dirty = PR_TRUE;
 
     replica_unlock(r->repl_lock);
 }
@@ -860,9 +858,6 @@ replica_update_ruv(Replica *r, const CSN *updated_csn, const char *replica_purl)
                                                     "to update RUV for replica %s, csn = %s\n",
                                   slapi_sdn_get_dn(r->repl_root),
                                   csn_as_string(updated_csn, PR_FALSE, csn_str));
-                } else {
-                    /* RUV updated - mark as dirty */
-                    r->repl_ruv_dirty = PR_TRUE;
                 }
             } else {
                 slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name,
@@ -1347,8 +1342,6 @@ replica_dump(Replica *r)
     slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name, "\tupdate dn: %s\n",
                   updatedn_list ? updatedn_list : "not configured");
     slapi_ch_free_string(&updatedn_list);
-    slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name, "\truv: %s configured and is %sdirty\n",
-                  r->repl_ruv ? "" : "not", r->repl_ruv_dirty ? "" : "not ");
     slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name, "\tCSN generator: %s configured\n",
                   r->repl_csngen ? "" : "not");
     /* JCMREPL - Dump Referrals */
@@ -1675,7 +1668,6 @@ replica_check_for_data_reload(Replica *r, void *arg __attribute__((unused)))
 
                     ruv_force_csn_update_from_ruv(upper_bound_ruv, r_ruv,
                                                   "Force update of database RUV (from CL RUV) -> ", SLAPI_LOG_NOTICE);
-                    replica_set_ruv_dirty(r);
                 }
 
             } else {
@@ -2778,11 +2770,6 @@ replica_write_ruv(Replica *r)
 
     replica_lock(r->repl_lock);
 
-    if (!r->repl_ruv_dirty) {
-        replica_unlock(r->repl_lock);
-        return rc;
-    }
-
     PR_ASSERT(r->repl_ruv);
 
     ruv_to_smod((RUV *)object_get_data(r->repl_ruv), &smod);
@@ -2817,14 +2804,10 @@ replica_write_ruv(Replica *r)
     /* ruv does not exist - create one */
     replica_lock(r->repl_lock);
 
-    if (rc == LDAP_SUCCESS) {
-        r->repl_ruv_dirty = PR_FALSE;
-    } else if (rc == LDAP_NO_SUCH_OBJECT) {
+    if (rc == LDAP_NO_SUCH_OBJECT) {
         /* this includes an internal operation - but since this only happens
            during server startup - its ok that we have lock around it */
         rc = _replica_configure_ruv(r, PR_TRUE);
-        if (rc == 0)
-            r->repl_ruv_dirty = PR_FALSE;
     } else /* error */
     {
         slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name,
@@ -3325,7 +3308,6 @@ replica_create_ruv_tombstone(Replica *r)
 
             if (ruv_init_new(csnstr, r->repl_rid, purl, &ruv) == RUV_SUCCESS) {
                 r->repl_ruv = object_new((void *)ruv, (FNFree)ruv_destroy);
-                r->repl_ruv_dirty = PR_TRUE;
                 return_value = LDAP_SUCCESS;
             } else {
                 slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_create_ruv_tombstone - "
@@ -3365,8 +3347,6 @@ replica_create_ruv_tombstone(Replica *r)
     slapi_add_internal_pb(pb);
     e = NULL; /* add consumes e, upon success or failure */
     slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &return_value);
-    if (return_value == LDAP_SUCCESS)
-        r->repl_ruv_dirty = PR_FALSE;
 
 done:
     slapi_entry_free(e);
@@ -3630,7 +3610,6 @@ replica_strip_cleaned_rids(Replica *r)
     ruv_get_cleaned_rids(ruv, rid);
     while (rid[i] != 0) {
         ruv_delete_replica(ruv, rid[i]);
-        replica_set_ruv_dirty(r);
         if (replica_write_ruv(r)) {
             slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name,
                           "replica_strip_cleaned_rids - Failed to write RUV\n");
@@ -3744,15 +3723,6 @@ replica_update_ruv_consumer(Replica *r, RUV *supplier_ruv)
     }
 }
 
-void
-replica_set_ruv_dirty(Replica *r)
-{
-    PR_ASSERT(r);
-    replica_lock(r->repl_lock);
-    r->repl_ruv_dirty = PR_TRUE;
-    replica_unlock(r->repl_lock);
-}
-
 PRBool
 replica_is_state_flag_set(Replica *r, int32_t flag)
 {

ldap/servers/plugins/replication/repl5_replica_config.c

@@ -937,7 +937,6 @@ replica_config_change_type_and_id(Replica *r, const char *new_type, const char *
                     replica_reset_csn_pl(r);
                 }
                 ruv_delete_replica(ruv, oldrid);
-                replica_set_ruv_dirty(r);
                 cl5CleanRUV(oldrid);
                 replica_set_csn_assigned(r);
             }
@@ -1323,7 +1322,6 @@ replica_execute_cleanruv_task(Object *r, ReplicaId rid, char *returntext __attri
         return LDAP_UNWILLING_TO_PERFORM;
     }
     rc = ruv_delete_replica(local_ruv, rid);
-    replica_set_ruv_dirty(replica);
     if (replica_write_ruv(replica)) {
         slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name, "cleanAllRUV_task - Could not write RUV\n");
     }