From 127e0d954eb7741c4afdc0305f7970b7ea164e8d Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz <lkrispen@redhat.com>
Date: Thu, 9 Nov 2017 11:28:34 +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: William,Thierry - Thanks
---
ldap/servers/plugins/replication/cl5_api.c | 39 ++++++++++++++++++++++
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, 40 insertions(+), 34 deletions(-)
diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c
index ec648c014..55032dfb0 100644
--- a/ldap/servers/plugins/replication/cl5_api.c
+++ b/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);
diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h
index c6e79b7e2..4e206a0fc 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 e5296bf1c..77f4f18e4 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 9c8d6adbb..e025f34d8 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.13.6