On 12.11.2013 16:13, Petr Spacek wrote:
On 5.11.2013 12:29, Tomas Hozza wrote:
----- Original Message -----
Hello,

Improve performance of initial LDAP synchronization.

Changes are not journaled and SOA serial is not incremented during initial
LDAP synchronization.

This eliminates unnecessary synchronous writes to journal and also
unnecessary SOA serial writes to LDAP.

See commit messages and comments in syncrepl.c for all the gory details.


ACK.

Patches look good. AXFR and IXFR works as expected. Also BIND starts up much
faster with these patches. Good job... :)

Regards,

Tomas

Hmm, further testing revealed that patch 203 changed behavior little bit:
Zones were loaded from LDAP correctly, but the SOA serial wasn't changed at
all. As a result, zone transfers return inconsistent results if the data in
LDAP are changed when BIND was not running.

Patch 203-v2 imitates the old behavior from bind-dyndb-ldap 3.x: Zone serial
is bumped *once* for each zone, so any changed in LDAP will be transferred
correctly (with new serial).

Patch 202 v2 was rebased and fixes reconnection to LDAP and solves deadlock caused by too eager locking.

Patch 203 v3 was rebased and fixes reconnection to LDAP.

These patches should go to master branch.

--
Petr^2 Spacek

From b73e345393d55fe411875d52e6fe4c98e1de8c04 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 9 Dec 2013 11:11:10 +0100
Subject: [PATCH] Detect end of initial LDAP synchronization phase.

LDAP intermediate message with refreshDone = TRUE is translated to
sync_barrier_wait() call. This call sends 'barrier event' to all tasks
involved in syncrepl event processing. The call will return when all tasks
have processed all events enqueued before the call.

Effectively, all events produced by initial LDAP synchronization
are processed first. Current state of synchronization is available via
sync_state_get() call.

See comments in syncrepl.c for all the gory details.

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/Makefile.am   |   2 +
 src/ldap_helper.c |  67 +++++++++--
 src/ldap_helper.h |   2 +
 src/syncrepl.c    | 351 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/syncrepl.h    |  63 ++++++++++
 5 files changed, 473 insertions(+), 12 deletions(-)
 create mode 100644 src/syncrepl.c
 create mode 100644 src/syncrepl.h

diff --git a/src/Makefile.am b/src/Makefile.am
index cff074eabbe513fe7a7483fab7b5555eec0c8471..6856957f48dbe32750009ab8a32487add05d5c1c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -16,6 +16,7 @@ HDRS =				\
 	rdlist.h		\
 	semaphore.h		\
 	settings.h		\
+	syncrepl.h		\
 	str.h			\
 	types.h			\
 	util.h			\
@@ -37,6 +38,7 @@ ldap_la_SOURCES =		\
 	rdlist.c		\
 	semaphore.c		\
 	settings.c		\
+	syncrepl.c		\
 	str.c			\
 	zone_manager.c		\
 	zone_register.c
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b086272c555dc1ca9e0151c3b481e06f2b6d93b4..4f87d23086843c08ee2de9ab42a86e484325a544 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -81,6 +81,7 @@
 #include "semaphore.h"
 #include "settings.h"
 #include "str.h"
+#include "syncrepl.h"
 #include "util.h"
 #include "zone_manager.h"
 #include "zone_register.h"
@@ -168,6 +169,8 @@ struct ldap_instance {
 	settings_set_t		*local_settings;
 	settings_set_t		*global_settings;
 	dns_forwarders_t	orig_global_forwarders; /* from named.conf */
+
+	sync_ctx_t		*sctx;
 };
 
 struct ldap_pool {
@@ -215,8 +218,7 @@ const ldap_auth_pair_t supported_ldap_auth[] = {
 	{ AUTH_INVALID, NULL		},
 };
 
-#define LDAPDB_EVENTCLASS 	ISC_EVENTCLASS(0xDDDD)
-#define LDAPDB_EVENT_SYNCREPL	(LDAPDB_EVENTCLASS + 0)
+#define LDAPDB_EVENT_SYNCREPL_UPDATE	(LDAPDB_EVENTCLASS + 1)
 
 typedef struct ldap_syncreplevent ldap_syncreplevent_t;
 struct ldap_syncreplevent {
@@ -529,6 +531,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
 	ISC_LIST_INIT(ldap_inst->orig_global_forwarders.addrs);
 	ldap_inst->task = task;
 	ldap_inst->watcher = 0;
+	CHECK(sync_ctx_init(ldap_inst->mctx, task, &ldap_inst->sctx));
 
 	isc_string_printf_truncate(settings_name, PRINT_BUFF_SIZE,
 				   SETTING_SET_NAME_LOCAL " for database %s",
@@ -653,6 +656,8 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
 	settings_set_free(&ldap_inst->global_settings);
 	settings_set_free(&ldap_inst->local_settings);
 
+	sync_ctx_free(&ldap_inst->sctx);
+
 	MEM_PUT_AND_DETACH(ldap_inst);
 
 	*ldap_instp = NULL;
@@ -776,6 +781,8 @@ create_zone(ldap_instance_t *ldap_inst, dns_name_t *name, dns_zone_t **zonep)
 	isc_result_t result;
 	dns_zone_t *zone = NULL;
 	const char *argv[2];
+	sync_state_t sync_state;
+	isc_task_t *task = NULL;
 
 	REQUIRE(ldap_inst != NULL);
 	REQUIRE(name != NULL);
@@ -820,13 +827,24 @@ create_zone(ldap_instance_t *ldap_inst, dns_name_t *name, dns_zone_t **zonep)
 	dns_zone_setclass(zone, dns_rdataclass_in);
 	dns_zone_settype(zone, dns_zone_master);
 	CHECK(dns_zone_setdbtype(zone, 2, argv));
+	CHECK(dns_zonemgr_managezone(ldap_inst->zmgr, zone));
+	sync_state_get(ldap_inst->sctx, &sync_state);
+	if (sync_state == sync_init) {
+		dns_zone_gettask(zone, &task);
+		CHECK(sync_task_add(ldap_inst->sctx, task));
+		isc_task_detach(&task);
+	}
 
 	*zonep = zone;
 	return ISC_R_SUCCESS;
 
 cleanup:
+	if (dns_zone_getmgr(zone) != NULL)
+		dns_zonemgr_releasezone(ldap_inst->zmgr, zone);
 	if (zone != NULL)
 		dns_zone_detach(&zone);
+	if (task != NULL)
+		isc_task_detach(&task);
 
 	return result;
 }
@@ -846,15 +864,9 @@ publish_zone(ldap_instance_t *inst, dns_zone_t *zone)
 	}
 
 	dns_zone_setview(zone, inst->view);
-	result = dns_zonemgr_managezone(inst->zmgr, zone);
-	if (result != ISC_R_SUCCESS)
-		return result;
 	CHECK(dns_view_addzone(inst->view, zone));
 
 cleanup:
-	if (result != ISC_R_SUCCESS)
-		dns_zonemgr_releasezone(inst->zmgr, zone);
-
 	if (freeze)
 		dns_view_freeze(inst->view);
 
@@ -4093,6 +4105,7 @@ syncrepl_update(ldap_instance_t *inst, ldap_entry_t *entry, int chgtype)
 	isc_mem_t *mctx = NULL;
 	isc_taskaction_t action = NULL;
 	isc_task_t *task = NULL;
+	sync_state_t sync_state;
 
 	log_debug(20, "syncrepl change type: " /*"none%d,"*/ "add%d, del%d, mod%d", /* moddn%d", */
 		  /* !SYNCREPL_ANY(chgtype), */ SYNCREPL_ADD(chgtype),
@@ -4174,8 +4187,16 @@ syncrepl_update(ldap_instance_t *inst, ldap_entry_t *entry, int chgtype)
 		goto cleanup;
 	}
 
+	/* All events for single zone are handled by one task, so we don't
+	 * need to spend time with normal records. */
+	if (action == update_zone || action == update_config) {
+		sync_state_get(inst->sctx, &sync_state);
+		if (sync_state == sync_init)
+			CHECK(sync_task_add(inst->sctx, task));
+	}
+
 	pevent = (ldap_syncreplevent_t *)isc_event_allocate(inst->mctx,
-				inst, LDAPDB_EVENT_SYNCREPL,
+				inst, LDAPDB_EVENT_SYNCREPL_UPDATE,
 				action, NULL,
 				sizeof(ldap_syncreplevent_t));
 
@@ -4313,6 +4334,9 @@ int ldap_sync_search_entry (
 	/* TODO: Use this for UUID->DN mapping and MODDN detection. */
 	UNUSED(entryUUID);
 
+	if (inst->exiting)
+		return LDAP_SUCCESS;
+
 
 	CHECK(ldap_entry_create(inst->mctx, ls->ls_ld, msg, &entry));
 	syncrepl_update(inst, entry, phase);
@@ -4333,7 +4357,7 @@ cleanup:
 	return LDAP_SUCCESS;
 }
 
-/*
+/**
  * Called when specific intermediate/final messages are returned
  * by ldap_sync_init()/ldap_sync_poll().
  * If phase is LDAP_SYNC_CAPI_PRESENTS or LDAP_SYNC_CAPI_DELETES,
@@ -4346,19 +4370,35 @@ cleanup:
  * If phase is LDAP_SYNC_CAPI_PRESENTS_IDSET or
  * LDAP_SYNC_CAPI_DELETES_IDSET, syncUUIDs is an array of UUIDs
  * that are either present or have been deleted.
+ *
+ * @see Section @ref syncrepl-theory in syncrepl.c for the background.
  */
 int ldap_sync_intermediate (
 	ldap_sync_t			*ls,
 	LDAPMessage			*msg,
 	BerVarray			syncUUIDs,
 	ldap_sync_refresh_t		phase ) {
 
-	UNUSED(ls);
+	isc_result_t	result;
+	ldap_instance_t *inst = ls->ls_private;
+
 	UNUSED(msg);
 	UNUSED(syncUUIDs);
 	UNUSED(phase);
 
-	log_error("ldap_sync_intermediate is not yet handled");
+	if (inst->exiting)
+		return LDAP_SUCCESS;
+
+	if (phase == LDAP_SYNC_CAPI_DONE) {
+		log_debug(1, "ldap_sync_intermediate RECEIVED");
+		result = sync_barrier_wait(inst->sctx, inst->db_name);
+		if (result != ISC_R_SUCCESS)
+			log_error_r("sync_barrier_wait() failed for instance '%s'",
+				    inst->db_name);
+		else
+			log_info("all zones from LDAP instance '%s' loaded",
+				 inst->db_name);
+	}
 	return LDAP_SUCCESS;
 }
 
@@ -4405,8 +4445,11 @@ ldap_sync_prepare(ldap_instance_t *inst, settings_set_t *settings,
 	isc_uint32_t reconnect_interval;
 	ldap_sync_t *ldap_sync = NULL;
 
+	REQUIRE(inst != NULL);
 	REQUIRE(ldap_syncp != NULL && *ldap_syncp == NULL);
 
+	sync_state_reset(inst->sctx);
+
 	/* Try to connect. */
 	while (conn->handle == NULL) {
 		result = ISC_R_SHUTTINGDOWN;
diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index 7f85dafa9e073555b381401b2d946738cce86c78..cbed09c0edad4ed4e2cf8ba3420d2c75a442ab83 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -27,6 +27,8 @@
 
 #include <isc/util.h>
 
+#define LDAPDB_EVENTCLASS 		ISC_EVENTCLASS(0xDDDD)
+
 typedef struct ldap_instance	ldap_instance_t;
 
 isc_result_t ldapdb_rdatalist_findrdatatype(ldapdb_rdatalist_t *rdatalist,
diff --git a/src/syncrepl.c b/src/syncrepl.c
new file mode 100644
index 0000000000000000000000000000000000000000..b1a74617292aa7afeb4c012f50bbc26700021122
--- /dev/null
+++ b/src/syncrepl.c
@@ -0,0 +1,351 @@
+/*
+ * Authors: Petr Spacek <pspa...@redhat.com>
+ *
+ * Copyright (C) 2013 Red Hat
+ * see file 'COPYING' for use and warranty information
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 or later
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <unistd.h>
+
+#include <isc/condition.h>
+#include <isc/event.h>
+#include <isc/mutex.h>
+#include <isc/task.h>
+#include <isc/util.h>
+
+#include "ldap_helper.h"
+#include "util.h"
+#include "syncrepl.h"
+#include "zone_manager.h"
+
+#define LDAPDB_EVENT_SYNCREPL_BARRIER	(LDAPDB_EVENTCLASS + 2)
+
+typedef struct task_element task_element_t;
+struct task_element {
+	isc_task_t			*task;
+	ISC_LINK(task_element_t)	link;
+};
+
+/**
+ * @brief Synchronisation context.
+ *
+ * This structure provides information necessary for detecting the end
+ * of initial LDAP synchronization.
+ *
+ * @section syncrepl-theory RFC 4533 and bind-dyndb-ldap theory
+ * LDAP delivers RFC 4533 messages via ldap_sync_init()
+ * and ldap_sync_poll() calls. Each LDAP message is translated
+ * by syncrepl_update() to an ISC event. This new event is sent to the task
+ * associated with LDAP instance or to the task associated with particular DNS
+ * zone. Each task involved in event processing is added to task list in
+ * struct sync_ctx by sync_task_add() call.
+ *
+ * The initial synchronization is done when LDAP intermediate message
+ * (with attribute refreshDone = TRUE) was received and all events generated
+ * before this message were processed.
+ *
+ * LDAP intermediate message handler ldap_sync_intermediate() calls
+ * sync_barrier_wait() and it sends sync_barrierev event to all involved tasks.
+ * sync_barrier_wait() returns only if all tasks processed all sync_barrierev
+ * events. As a result, all events generated before sync_barrier_wait() call
+ * are processed before the call returns.
+ *
+ * @warning There are two assumptions:
+ * 	@li Each task processes events in FIFO order.
+ * 	@li The task assigned to a LDAP instance or a DNS zone never changes.
+ *
+ * @see ldap_sync_search_entry()
+ */
+struct sync_ctx {
+	isc_refcount_t			task_cnt; /**< provides atomic access */
+	isc_mem_t			*mctx;
+
+	isc_mutex_t			mutex;	/**< guards rest of the structure */
+	isc_condition_t			cond;	/**< for signal when task_cnt == 0 */
+	sync_state_t			state;
+	ISC_LIST(task_element_t)	tasks;	/**< list of tasks processing
+						     events from initial
+						     synchronization phase */
+};
+
+/**
+ * @brief This event is used to separate event queue for particular task to
+ * part 'before' and 'after' this event.
+ *
+ * This is auxiliary event supporting sync_barrier_wait().
+ *
+ * @todo Solution with inst_name is not very clever. Reference counting would
+ *       be much better, but ldap_instance_t doesn't support reference counting.
+ */
+struct sync_barrierev {
+	ISC_EVENT_COMMON(sync_barrierev_t);
+	const char	*dbname;
+	sync_ctx_t	*sctx;
+};
+
+/**
+ * @brief Event handler for 'sync barrier event'.
+ *
+ * This is auxiliary event handler for events created by
+ * sync_barrierev_create() and sent by sync_barrier_wait().
+ *
+ * Each call decrements task_cnt counter in synchronization context associated
+ * with the particular event. Broadcast will be send to condition in associated
+ * synchronization context when task_cnt == 0.
+ */
+void
+barrier_decrement(isc_task_t *task, isc_event_t *event) {
+	isc_result_t result = ISC_R_SUCCESS;
+	ldap_instance_t *inst = NULL;
+	sync_barrierev_t *bev = NULL;
+	isc_uint32_t cnt;
+
+	UNUSED(task);
+	REQUIRE(event != NULL);
+
+	bev = (sync_barrierev_t *)event;
+	CHECK(manager_get_ldap_instance(bev->dbname, &inst));
+	isc_refcount_decrement(&bev->sctx->task_cnt, &cnt);
+	if (cnt == 0) {
+		log_debug(1, "sync_barrier_wait(): barrier reached");
+		LOCK(&bev->sctx->mutex);
+		REQUIRE(bev->sctx->state == sync_barrier);
+		bev->sctx->state = sync_finished;
+		isc_condition_broadcast(&bev->sctx->cond);
+		UNLOCK(&bev->sctx->mutex);
+	}
+
+cleanup:
+	if (result != ISC_R_SUCCESS)
+		log_error_r("barrier_decrement() failed");
+	isc_event_free(&event);
+	return;
+}
+
+static isc_result_t ATTR_NONNULLS
+sync_barrierev_create(sync_ctx_t *sctx, const char *inst_name,
+		      sync_barrierev_t **evp) {
+	sync_barrierev_t *ev = NULL;
+
+	REQUIRE(sctx != NULL);
+	REQUIRE(inst_name != NULL);
+	REQUIRE(evp != NULL && *evp == NULL);
+
+	ev = (sync_barrierev_t *)isc_event_allocate(sctx->mctx,
+				sctx, LDAPDB_EVENT_SYNCREPL_BARRIER,
+				barrier_decrement, NULL,
+				sizeof(sync_barrierev_t));
+	if (ev == NULL)
+		return ISC_R_NOMEMORY;
+
+	ev->dbname = inst_name;
+	ev->sctx = sctx;
+	*evp = ev;
+
+	return ISC_R_SUCCESS;
+}
+
+/**
+ * Initialize synchronization context.
+ *
+ * @param[in]	task	Task used for first synchronization events.
+ * 			Typically the ldap_inst->task.
+ * @param[out]	sctxp	The new synchronization context.
+ *
+ * @post state == sync_init
+ * @post task_cnt == 1
+ * @post tasks list contains the task
+ */
+isc_result_t
+sync_ctx_init(isc_mem_t *mctx, isc_task_t *task, sync_ctx_t **sctxp) {
+	isc_result_t result;
+	sync_ctx_t *sctx = NULL;
+	isc_boolean_t lock_ready = ISC_FALSE;
+	isc_boolean_t cond_ready = ISC_FALSE;
+	isc_boolean_t refcount_ready = ISC_FALSE;
+
+	REQUIRE(sctxp != NULL && *sctxp == NULL);
+	REQUIRE(ISCAPI_TASK_VALID(task));
+
+	CHECKED_MEM_GET_PTR(mctx, sctx);
+	ZERO_PTR(sctx);
+	isc_mem_attach(mctx, &sctx->mctx);
+
+	CHECK(isc_mutex_init(&sctx->mutex));
+	lock_ready = ISC_TRUE;
+	CHECK(isc_condition_init(&sctx->cond));
+	cond_ready = ISC_TRUE;
+
+	/* refcount includes ldap_inst->task implicitly */
+	CHECK(isc_refcount_init(&sctx->task_cnt, 0));
+	refcount_ready = ISC_TRUE;
+
+	ISC_LIST_INIT(sctx->tasks);
+
+	sctx->state = sync_init;
+	CHECK(sync_task_add(sctx, task));
+
+	*sctxp = sctx;
+	return ISC_R_SUCCESS;
+
+cleanup:
+	if (lock_ready == ISC_TRUE)
+		isc_mutex_destroy(&(*sctxp)->mutex);
+	if (cond_ready == ISC_TRUE)
+		isc_condition_init(&(*sctxp)->cond);
+	if (refcount_ready == ISC_TRUE)
+		isc_refcount_destroy(&(*sctxp)->task_cnt);
+	MEM_PUT_AND_DETACH(*sctxp);
+	return result;
+}
+
+void
+sync_ctx_free(sync_ctx_t **sctxp) {
+	sync_ctx_t *sctx = NULL;
+	task_element_t *taskel = NULL;
+	task_element_t *next_taskel = NULL;
+
+	REQUIRE(sctxp != NULL);
+
+	if (*sctxp == NULL)
+		return;
+
+	sctx = *sctxp;
+
+	/* detach all tasks in task list, decrement refcounter to zero and
+	 * deallocate whole task list */
+	LOCK(&sctx->mutex);
+	for (taskel = next_taskel = HEAD(sctx->tasks);
+	     taskel != NULL;
+	     taskel = next_taskel) {
+		next_taskel = NEXT(taskel, link);
+		UNLINK(sctx->tasks, taskel, link);
+		isc_task_detach(&taskel->task);
+		isc_refcount_decrement(&sctx->task_cnt, NULL);
+		SAFE_MEM_PUT_PTR(sctx->mctx, taskel);
+	}
+	isc_condition_destroy(&sctx->cond);
+	isc_refcount_destroy(&sctx->task_cnt);
+	UNLOCK(&sctx->mutex);
+
+	isc_mutex_destroy(&(*sctxp)->mutex);
+	MEM_PUT_AND_DETACH(*sctxp);
+}
+
+void
+sync_state_get(sync_ctx_t *sctx, sync_state_t *statep) {
+	REQUIRE(sctx != NULL);
+
+	LOCK(&sctx->mutex);
+	*statep = sctx->state;
+	UNLOCK(&sctx->mutex);
+}
+
+void
+sync_state_reset(sync_ctx_t *sctx) {
+	REQUIRE(sctx != NULL);
+
+	LOCK(&sctx->mutex);
+	sctx->state = sync_init;
+	UNLOCK(&sctx->mutex);
+}
+
+/**
+ * @brief Add task to task list in synchronization context.
+ *
+ * As a result, subsequent sync_barrier_wait() call will wait until all events
+ * queued for the task are processed.
+ */
+isc_result_t
+sync_task_add(sync_ctx_t *sctx, isc_task_t *task) {
+	isc_result_t result = ISC_R_SUCCESS;
+	isc_uint32_t cnt;
+	task_element_t *newel = NULL;
+
+	REQUIRE(sctx != NULL);
+	REQUIRE(sctx->state == sync_init);
+	REQUIRE(ISCAPI_TASK_VALID(task));
+
+	CHECKED_MEM_GET_PTR(sctx->mctx, newel);
+	ZERO_PTR(newel);
+	ISC_LINK_INIT(newel, link);
+	newel->task = NULL;
+	isc_task_attach(task, &newel->task);
+
+	LOCK(&sctx->mutex);
+	ISC_LIST_APPEND(sctx->tasks, newel, link);
+	isc_refcount_increment0(&sctx->task_cnt, &cnt);
+	UNLOCK(&sctx->mutex);
+
+	log_debug(2, "adding task %p to syncrepl list; %u tasks in list",
+		  task, cnt);
+
+cleanup:
+	return result;
+}
+
+/**
+ * Wait until all tasks in sctx->tasks list process all events enqueued
+ * before sync_barrier_wait() call.
+ *
+ * @param[in,out]	sctx		Synchronization context
+ * @param[in]		inst_name	LDAP instance name for given sctx
+ *
+ * @pre  sctx->state == sync_init
+ * @post sctx->state == sync_finished and all tasks processed all events
+ *       enqueued before sync_barrier_wait() call.
+ */
+isc_result_t
+sync_barrier_wait(sync_ctx_t *sctx, const char *inst_name) {
+	isc_result_t result;
+	isc_event_t *ev = NULL;
+	sync_barrierev_t *bev = NULL;
+	task_element_t *taskel = NULL;
+	task_element_t *next_taskel = NULL;
+
+	LOCK(&sctx->mutex);
+	REQUIRE(sctx->state == sync_init);
+	if (EMPTY(sctx->tasks)) {
+		log_bug("sync_barrier_wait(): called with empty task list");
+		sctx->state = sync_finished;
+		CLEANUP_WITH(ISC_R_SUCCESS);
+	}
+
+	sctx->state = sync_barrier;
+	for (taskel = next_taskel = HEAD(sctx->tasks);
+	     taskel != NULL;
+	     taskel = next_taskel) {
+		bev = NULL;
+		CHECK(sync_barrierev_create(sctx, inst_name, &bev));
+		next_taskel = NEXT(taskel, link);
+		UNLINK(sctx->tasks, taskel, link);
+		ev = (isc_event_t *)bev;
+		isc_task_sendanddetach(&taskel->task, &ev);
+		SAFE_MEM_PUT_PTR(sctx->mctx, taskel);
+	}
+
+	log_debug(1, "sync_barrier_wait(): wait until all events are processed");
+	while (sctx->state != sync_finished)
+		isc_condition_wait(&sctx->cond, &sctx->mutex);
+	log_debug(1, "sync_barrier_wait(): all events were processed");
+
+cleanup:
+	UNLOCK(&sctx->mutex);
+
+	if (ev != NULL)
+		isc_event_free(&ev);
+	return result;
+}
diff --git a/src/syncrepl.h b/src/syncrepl.h
new file mode 100644
index 0000000000000000000000000000000000000000..c0f707c96e3a66bf2cb9013182911cc7666783bb
--- /dev/null
+++ b/src/syncrepl.h
@@ -0,0 +1,63 @@
+/*
+ * Authors: Petr Spacek <pspa...@redhat.com>
+ *
+ * Copyright (C) 2013 Red Hat
+ * see file 'COPYING' for use and warranty information
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 or later
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef SYNCREPL_H_
+#define SYNCREPL_H_
+
+/**
+ * SyncRepl state is stored inside ldap_instance_t.
+ * Attributes in ldap_instance_t are be modified in new_ldap_instance function,
+ * which means server is started or reloaded (running single-thread).
+ * Before modifying at other places, switch to single-thread mode via
+ * isc_task_beginexclusive() and then return back via isc_task_endexclusive()!
+ */
+typedef struct sync_ctx		sync_ctx_t;
+typedef enum sync_state		sync_state_t;
+typedef struct sync_barrierev	sync_barrierev_t;
+
+enum sync_state {
+	sync_init,	/**< initial synchronisation in progress;
+			     expecting LDAP intermediate message
+			     with refreshDone = TRUE */
+	sync_barrier,	/**< waiting until all tasks process events generated
+			     during initial synchronisation phase*/
+	sync_finished	/**< initial synchronisation done; all events generated
+			     during initial synchronisation were processed */
+};
+
+isc_result_t
+sync_ctx_init(isc_mem_t *mctx, isc_task_t *task, sync_ctx_t **sctxp);
+
+void
+sync_ctx_free(sync_ctx_t **statep);
+
+void
+sync_state_get(sync_ctx_t *sctx, sync_state_t *statep);
+
+void
+sync_state_reset(sync_ctx_t *sctx);
+
+isc_result_t
+sync_task_add(sync_ctx_t *sctx, isc_task_t *task);
+
+isc_result_t
+sync_barrier_wait(sync_ctx_t *sctx, const char *inst_name);
+
+#endif /* SYNCREPL_H_ */
-- 
1.8.3.1

From 9ec4e5ea76f36f808d6b2da05b47f2fa433b1d6c Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 9 Dec 2013 13:48:22 +0100
Subject: [PATCH] Improve performance of initial LDAP synchronization.

Changes are not journaled and SOA serial is not incremented until
LDAP synchronization reaches state "sync_finished".

This eliminates unnecessary synchronous writes to journal and also
unnecessary SOA serial writes to LDAP.

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/ldap_helper.c | 98 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 53 insertions(+), 45 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 4f87d23086843c08ee2de9ab42a86e484325a544..a5bebdced70fefe25e3a3779ee024fdd37d3cdd0 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1641,6 +1641,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 	dns_difftuple_t *soa_tuple = NULL;
 	isc_boolean_t soa_tuple_alloc = ISC_FALSE;
 
+	sync_state_t sync_state;
 	dns_journal_t *journal = NULL;
 	char *journal_filename = NULL;
 
@@ -1789,9 +1790,11 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 
 	dns_db_detachnode(rbtdb, &node);
 
-	/* Detect if SOA serial is affected by the update or not. */
+	/* Detect if SOA serial is affected by the update or not.
+	 * Always bump serial in case of re-synchronization. */
+	sync_state_get(inst->sctx, &sync_state);
 	CHECK(diff_analyze_serial(&diff, &soa_tuple, &data_changed));
-	if (data_changed == ISC_TRUE) {
+	if (data_changed == ISC_TRUE || sync_state != sync_finished) {
 		if (soa_tuple == NULL) {
 			/* The diff doesn't contain new SOA serial
 			 * => generate new serial and write it back to LDAP. */
@@ -1805,10 +1808,12 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 			CHECK(update_soa_serial(dns_updatemethod_unixtime,
 						soa_tuple, &new_serial));
 			dns_diff_appendminimal(&diff, &soa_tuple);
-		} else if (isc_serial_le(dns_soa_getserial(&soa_tuple->rdata),
-					 curr_serial)) {
+		} else if (sync_state != sync_finished ||
+			   isc_serial_le(dns_soa_getserial(&soa_tuple->rdata),
+					 curr_serial) || publish == ISC_TRUE) {
 			/* The diff tries to send SOA serial back!
-			 * => generate new serial and write it back to LDAP. */
+			 * => generate new serial and write it back to LDAP.
+			 * Force serial update if we are adding a new zone. */
 			ldap_writeback = ISC_TRUE;
 			CHECK(update_soa_serial(dns_updatemethod_unixtime,
 						soa_tuple, &new_serial));
@@ -1849,15 +1854,17 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 	}
 
 	if (!EMPTY(diff.tuples)) {
-		/* write the transaction to journal */
-		dns_zone_getraw(zone, &zone_raw);
-		if (zone_raw == NULL)
-			journal_filename = dns_zone_getjournal(zone);
-		else
-			journal_filename = dns_zone_getjournal(zone_raw);
-		CHECK(dns_journal_open(inst->mctx, journal_filename,
-				       DNS_JOURNAL_CREATE, &journal));
-		CHECK(dns_journal_write_transaction(journal, &diff));
+		if (sync_state == sync_finished) {
+			/* write the transaction to journal */
+			dns_zone_getraw(zone, &zone_raw);
+			if (zone_raw == NULL)
+				journal_filename = dns_zone_getjournal(zone);
+			else
+				journal_filename = dns_zone_getjournal(zone_raw);
+			CHECK(dns_journal_open(inst->mctx, journal_filename,
+					       DNS_JOURNAL_CREATE, &journal));
+			CHECK(dns_journal_write_transaction(journal, &diff));
+		}
 
 		/* commit */
 		CHECK(dns_diff_apply(&diff, rbtdb, version));
@@ -3822,6 +3829,7 @@ update_record(isc_task_t *task, isc_event_t *event)
 
 	dns_journal_t *journal = NULL;
 	char *journal_filename = NULL;
+	sync_state_t sync_state;
 
 	mctx = pevent->mctx;
 	dns_diff_init(mctx, &diff);
@@ -3849,11 +3857,6 @@ update_record(isc_task_t *task, isc_event_t *event)
 	CHECK(zr_get_zone_ptr(inst->zone_register, &origin, &zone_ptr));
 	zone_found = ISC_TRUE;
 
-	/* TODO: Do not bump serial during initial database dump. */
-	/* This optimization is disabled because we don't have reliable
-	 * detection if the initial database dump is finished.
-	 * if (!SYNCREPL_ANY(pevent->chgtype)) ... */
-
 update_restart:
 	rbtdb = NULL;
 	ldapdb = NULL;
@@ -3922,47 +3925,52 @@ update_restart:
 		dns_rdatasetiter_destroy(&rbt_rds_iterator);
 	}
 
+	sync_state_get(inst->sctx, &sync_state);
 	/* No real change in RR data -> do not increment SOA serial. */
 	if (HEAD(diff.tuples) != NULL) {
-		CHECK(dns_db_createsoatuple(ldapdb, version, mctx,
-					    DNS_DIFFOP_DEL, &soa_tuple));
-		dns_diff_append(&diff, &soa_tuple);
-		CHECK(dns_db_createsoatuple(ldapdb, version, mctx,
-					    DNS_DIFFOP_ADD, &soa_tuple));
-		CHECK(update_soa_serial(dns_updatemethod_unixtime,
-					soa_tuple, &serial));
-		dns_zone_log(zone_ptr, ISC_LOG_DEBUG(5),
-			     "writing new zone serial %u to LDAP", serial);
-		result = ldap_replace_serial(inst, &origin, serial);
-		if (result != ISC_R_SUCCESS)
-			dns_zone_log(zone_ptr, ISC_LOG_ERROR,
-				     "serial (%u) write back to LDAP failed",
-				     serial);
-		dns_diff_append(&diff, &soa_tuple);
+		if (sync_state == sync_finished) {
+			CHECK(dns_db_createsoatuple(ldapdb, version, mctx,
+						    DNS_DIFFOP_DEL, &soa_tuple));
+			dns_diff_append(&diff, &soa_tuple);
+			CHECK(dns_db_createsoatuple(ldapdb, version, mctx,
+						    DNS_DIFFOP_ADD, &soa_tuple));
+			CHECK(update_soa_serial(dns_updatemethod_unixtime,
+						soa_tuple, &serial));
+			dns_zone_log(zone_ptr, ISC_LOG_DEBUG(5),
+				     "writing new zone serial %u to LDAP", serial);
+			result = ldap_replace_serial(inst, &origin, serial);
+			if (result != ISC_R_SUCCESS)
+				dns_zone_log(zone_ptr, ISC_LOG_ERROR,
+					     "serial (%u) write back to LDAP failed",
+					     serial);
+			dns_diff_append(&diff, &soa_tuple);
+		}
 
 #if RBTDB_DEBUG >= 2
 		dns_diff_print(&diff, stdout);
 #else
 		dns_diff_print(&diff, NULL);
 #endif
-		/* write the transaction to journal */
-		dns_zone_getraw(zone_ptr, &zone_raw);
-		if (zone_raw == NULL)
-			journal_filename = dns_zone_getjournal(zone_ptr);
-		else
-			journal_filename = dns_zone_getjournal(zone_raw);
-		CHECK(dns_journal_open(mctx, journal_filename,
-				       DNS_JOURNAL_CREATE, &journal));
-		CHECK(dns_journal_write_transaction(journal, &diff));
-
+		if (sync_state == sync_finished) {
+			/* write the transaction to journal */
+			dns_zone_getraw(zone_ptr, &zone_raw);
+			if (zone_raw == NULL)
+				journal_filename = dns_zone_getjournal(zone_ptr);
+			else
+				journal_filename = dns_zone_getjournal(zone_raw);
+			CHECK(dns_journal_open(mctx, journal_filename,
+					       DNS_JOURNAL_CREATE, &journal));
+			CHECK(dns_journal_write_transaction(journal, &diff));
+		}
 		/* commit */
 		CHECK(dns_diff_apply(&diff, rbtdb, version));
 		dns_db_closeversion(ldapdb, &version, ISC_TRUE);
 	}
 
 	/* Check if the zone is loaded or not.
 	 * No other function above returns DNS_R_NOTLOADED. */
-	result = dns_zone_getserial2(zone_ptr, &serial);
+	if (sync_state == sync_finished)
+		result = dns_zone_getserial2(zone_ptr, &serial);
 
 cleanup:
 #ifdef RBTDB_DEBUG
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to