On 21.2.2014 19:35, Lukas Slebodnik wrote:
On (13/12/13 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.

--
Petr^2 Spacek


When I was testing upcoming bind-dyndb-ldap 4.0 release,
There was an interesting warning from clang static analyser.
I thought it was a false passitive, but it isn't.

Patch is attached


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


<snip>

+/**
+ * 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);
                              ^^^^^^^^^^^^^^
                   *sctxp is 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;
     ^^^^^^
value to *sctxp is asigned only on this line.

+       return ISC_R_SUCCESS;
+
+cleanup:
*sctxp will be NULL in cleanup section

+       if (lock_ready == ISC_TRUE)
+               isc_mutex_destroy(&(*sctxp)->mutex);
                           &(NULL)->mutex
                      It does not look like a good idea :-)
+       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;
+}
+

LS

ACK. Thank you for discovering this!

Pushed to master: e346fbce099eacb1cd860e0624dcaaea36161169

--
Petr^2 Spacek

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

Reply via email to