From 07a94f0033595d29f1a19cb8093cd0f33942d307 Mon Sep 17 00:00:00 2001 From: Ludwig Krispenz Date: Tue, 7 Nov 2017 15:02:39 +0100 Subject: [PATCH] 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: ? --- ldap/servers/plugins/replication/cl5_api.c | 38 ++++++++++++++++++++++ ldap/servers/plugins/replication/repl5.h | 1 - ldap/servers/plugins/replication/repl5_replica.c | 32 +----------------- .../plugins/replication/repl5_replica_config.c | 2 -- 4 files changed, 39 insertions(+), 34 deletions(-) diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c index ec648c0..68ed39e 100644 --- a/ldap/servers/plugins/replication/cl5_api.c +++ b/ldap/servers/plugins/replication/cl5_api.c @@ -250,6 +250,7 @@ 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 int _cl5CheckMaxRUV(CL5DBFile *file, RUV *maxruv); /* replay iteration */ #ifdef FOR_DEBUGGING @@ -2716,6 +2717,36 @@ _cl5WriteBervals(struct berval **bv, char **buff, u_int32_t *size) return CL5_SUCCESS; } +static int +_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 int +_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 +4041,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); diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h index c6e79b7..4e206a0 100644 --- a/ldap/servers/plugins/replication/repl5.h +++ b/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); diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c index e5296bf..77f4f18 100644 --- a/ldap/servers/plugins/replication/repl5_replica.c +++ b/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) { diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c index 9c8d6ad..e025f34 100644 --- a/ldap/servers/plugins/replication/repl5_replica_config.c +++ b/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"); } -- 2.5.5