Re: [Freeipa-devel] [PATCH 0202-0203] Improve performance of initial LDAP synchronizationDetect end of initial LDAP synchronization phase

2014-02-21 Thread Petr Spacek

On 13.12.2013 17:44, Petr Spacek wrote:

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.


Pushed to master branch:
218af8258536bcbe9aad515dcefa82ae671e1ddf
0c4b7584da340d80c5c64b238256d8616d0d85ba

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0202-0203] Improve performance of initial LDAP synchronizationDetect end of initial LDAP synchronization phase

2013-12-13 Thread Petr Spacek

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 cff074eabbe513fe7a7483fab7beec0c8471..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(0x)
-#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, 

Re: [Freeipa-devel] [PATCH 0202-0203] Improve performance of initial LDAP synchronizationDetect end of initial LDAP synchronization phase

2013-11-12 Thread Petr Spacek

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).


--
Petr^2 Spacek
From d856c94f16c9eeedab46e675e1e78701f4052928 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 31 Oct 2013 14:45:55 +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, except
for adding the zone apex.

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

There is only one LDAP write for each zone - the one-time serial
bump. This change is not journaled.

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

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 8e148534c9b7712102f78f9dbd960d4858da1091..a93469cb9e396a83b271a1a3c62056c488ac292d 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1631,6 +1631,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;
 
@@ -1796,9 +1797,10 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 		soa_tuple, new_serial));
 			dns_diff_appendminimal(diff, soa_tuple);
 		} else if (isc_serial_le(dns_soa_getserial(soa_tuple-rdata),
-	 curr_serial)) {
+	 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));
@@ -1823,6 +1825,10 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 		  * = do nothing. */
 	}
 
+	sync_state_get(inst-sctx, sync_state);
+	/* Allow write to LDAP if we are adding a new zone. */
+	if (publish == ISC_FALSE  sync_state != sync_finished)
+		ldap_writeback = ISC_FALSE;
 #if RBTDB_DEBUG = 2
 	dns_diff_print(diff, stdout);
 #else
@@ -1839,19 +1845,22 @@ 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));
-		dns_journal_destroy(journal);
+		if (sync_state == sync_finished)
+			dns_journal_destroy(journal);
 		dns_db_closeversion(rbtdb, version, ISC_TRUE);
 	}
 
@@ -3793,6 +3802,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);
@@ -3819,11 +3829,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.
-	 

Re: [Freeipa-devel] [PATCH 0202-0203] Improve performance of initial LDAP synchronizationDetect end of initial LDAP synchronization phase

2013-11-05 Thread Tomas Hozza
- 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

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


[Freeipa-devel] [PATCH 0202-0203] Improve performance of initial LDAP synchronizationDetect end of initial LDAP synchronization phase

2013-11-01 Thread Petr Spacek

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.

--
Petr^2 Spacek
From 9f17bc7678560eed4f2740c1a2d756e720a79936 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 31 Oct 2013 14:34:26 +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 |  56 ++---
 src/ldap_helper.h |   2 +
 src/syncrepl.c| 342 ++
 src/syncrepl.h|  60 ++
 5 files changed, 449 insertions(+), 13 deletions(-)
 create mode 100644 src/syncrepl.c
 create mode 100644 src/syncrepl.h

diff --git a/src/Makefile.am b/src/Makefile.am
index cff074eabbe513fe7a7483fab7beec0c8471..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 2f68b26bd0cefe097970039ddfd141539804d86a..8e148534c9b7712102f78f9dbd960d4858da1091 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(0x)
-#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;
@@ -820,11 +825,14 @@ 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));
 
 	*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);
 
@@ -846,15 +854,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);
 
@@ -4069,6 +4071,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),
@@ -4150,8 +4153,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) {