ldb_kv: Avoid memdup of database records in the case of base searches

This makes LDAP bind significantly faster in the case of having many
members, due to large size of these records (with tens of thousands of
member links). During the nested group calculation, you are only
interested in memberOf not the member links.

(We add a bit-field to determine whether or not the backend actually
supports pointing into database memory. For some reason TDB pointers
aren't stable, so for now we set this option just on LMDB backends.)

Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 6ba2a49300aacf0131d865c9336a638604af0033..b45142abe5c69e2c5fccb2274f555ffab300fef0 100644 (file)
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -560,6 +560,7 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
 #define LDB_UNPACK_DATA_FLAG_NO_DN           0x0002
 #define LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC 0x0004
 #define LDB_UNPACK_DATA_FLAG_NO_ATTRS        0x0008
+#define LDB_UNPACK_DATA_FLAG_READ_LOCKED     0x0010
 
 /**
  Forces a specific ldb handle to use the global event context.

diff --git a/lib/ldb/ldb_key_value/ldb_kv.h b/lib/ldb/ldb_key_value/ldb_kv.h
index 92106caae85a24c71ce0d08b17f069ac47edf817..778a8991bb8ae27b4d9767dab96ac540de169efc 100644 (file)
--- a/lib/ldb/ldb_key_value/ldb_kv.h
+++ b/lib/ldb/ldb_key_value/ldb_kv.h
@@ -13,6 +13,8 @@ typedef int (*ldb_kv_traverse_fn)(struct ldb_kv_private *ldb_kv,
                                  void *ctx);
 
 struct kv_db_ops {
+       uint32_t options;
+
        int (*store)(struct ldb_kv_private *ldb_kv,
                     struct ldb_val key,
                     struct ldb_val data,
@@ -175,6 +177,13 @@ struct ldb_kv_reindex_context {
 #define LDB_KV_GUID_SIZE 16
 #define LDB_KV_GUID_KEY_SIZE (LDB_KV_GUID_SIZE + sizeof(LDB_KV_GUID_KEY_PREFIX) - 1)
 
+/* LDB KV options */
+/*
+ * This allows pointers to be referenced after the callback to any variant of
+ * iterate or fetch_and_parse -- as long as an overall read lock is held.
+ */
+#define LDB_KV_OPTION_STABLE_READ_LOCK 0x00000001
+
 /*
  * The following definitions come from lib/ldb/ldb_key_value/ldb_kv_cache.c
  */

diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
index 350289a78e372a6a74770b5bff851a2bf8562da2..9f0de7d260e5e831c521920a6721aaf50d93201f 100644 (file)
--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
@@ -406,7 +406,13 @@ normal_index:
                                dn,
                                msg,
                                LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC |
-                                   LDB_UNPACK_DATA_FLAG_NO_DN);
+                               LDB_UNPACK_DATA_FLAG_NO_DN |
+                               /*
+                                * The entry point ldb_kv_search_indexed is
+                                * only called from the read-locked
+                                * ldb_kv_search.
+                                */
+                               LDB_UNPACK_DATA_FLAG_READ_LOCKED);
        if (ret != LDB_SUCCESS) {
                talloc_free(msg);
                return ret;
@@ -2222,7 +2228,13 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
                                      keys[i],
                                      msg,
                                      LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC |
-                                         LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
+                                     LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC |
+                                     /*
+                                      * The entry point ldb_kv_search_indexed is
+                                      * only called from the read-locked
+                                      * ldb_kv_search.
+                                      */
+                                     LDB_UNPACK_DATA_FLAG_READ_LOCKED);
                if (ret == LDB_ERR_NO_SUCH_OBJECT) {
                        /*
                         * the record has disappeared? yes, this can

diff --git a/lib/ldb/ldb_key_value/ldb_kv_search.c b/lib/ldb/ldb_key_value/ldb_kv_search.c
index aa086a88d9a6ce4e9e951223cd18b95b69014f18..a2946d6506b0392315b47defd3a98878becd1325 100644 (file)
--- a/lib/ldb/ldb_key_value/ldb_kv_search.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_search.c
@@ -190,22 +190,45 @@ static int ldb_kv_parse_data_unpack(struct ldb_val key,
        struct ldb_context *ldb = ldb_module_get_ctx(ctx->module);
        struct ldb_val data_parse = data;
 
-       if (ctx->unpack_flags & LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC) {
-               /*
-                * If we got LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC
-                * we need at least do a memdup on the whole
-                * data buffer as that may change later
-                * and the caller needs a stable result.
-                */
-               data_parse.data = talloc_memdup(ctx->msg,
-                                               data.data,
-                                               data.length);
-               if (data_parse.data == NULL) {
-                       ldb_debug(ldb, LDB_DEBUG_ERROR,
-                                 "Unable to allocate data(%d) for %*.*s\n",
-                                 (int)data.length,
-                                 (int)key.length, (int)key.length, key.data);
-                       return LDB_ERR_OPERATIONS_ERROR;
+       struct ldb_kv_private *ldb_kv =
+           talloc_get_type(ldb_module_get_private(ctx->module), struct ldb_kv_private);
+
+       if ((ctx->unpack_flags & LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC)) {
+               if ((ldb_kv->kv_ops->options & LDB_KV_OPTION_STABLE_READ_LOCK) &&
+                   (ctx->unpack_flags & LDB_UNPACK_DATA_FLAG_READ_LOCKED) &&
+                   !ldb_kv->kv_ops->transaction_active(ldb_kv)) {
+                       /*
+                        * In the case where no transactions are active and
+                        * we're in a read-lock, we can point directly into
+                        * database memory.
+                        *
+                        * The database can't be changed underneath us and we
+                        * will duplicate this data in the call to filter.
+                        *
+                        * This is seen in:
+                        * - ldb_kv_index_filter
+                        * - ldb_kv_search_and_return_base
+                        */
+               } else {
+                       /*
+                        * In every other case, if we got
+                        * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC we need at least
+                        * do a memdup on the whole data buffer as that may
+                        * change later and the caller needs a stable result.
+                        *
+                        * During transactions, pointers could change and in
+                        * TDB, there just aren't the same guarantees.
+                        */
+                       data_parse.data = talloc_memdup(ctx->msg,
+                                                       data.data,
+                                                       data.length);
+                       if (data_parse.data == NULL) {
+                               ldb_debug(ldb, LDB_DEBUG_ERROR,
+                                         "Unable to allocate data(%d) for %*.*s\n",
+                                         (int)data.length,
+                                         (int)key.length, (int)key.length, key.data);
+                               return LDB_ERR_OPERATIONS_ERROR;
+                       }
                }
        }
 
@@ -635,7 +658,8 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
                                ctx->base,
                                msg,
                                LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC |
-                                   LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
+                               LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC |
+                               LDB_UNPACK_DATA_FLAG_READ_LOCKED);
 
        if (ret == LDB_ERR_NO_SUCH_OBJECT) {
                if (ldb_kv->check_base == false) {

diff --git a/lib/ldb/ldb_mdb/ldb_mdb.c b/lib/ldb/ldb_mdb/ldb_mdb.c
index 01fa024e711245cfc53cfeb68e0e6541620dd9fb..68ee97acb644e79cba31ea9daddb5ff3fcf4a80f 100644 (file)
--- a/lib/ldb/ldb_mdb/ldb_mdb.c
+++ b/lib/ldb/ldb_mdb/ldb_mdb.c
@@ -739,9 +739,9 @@ static size_t lmdb_get_size(struct ldb_kv_private *ldb_kv)
        return stats.ms_entries;
 }
 
-
-
 static struct kv_db_ops lmdb_key_value_ops = {
+       .options            = LDB_KV_OPTION_STABLE_READ_LOCK,
+
        .store              = lmdb_store,
        .delete             = lmdb_delete,
        .iterate            = lmdb_traverse_fn,

diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 3dcc158729a1f27300ca7dbc357c7444c51333e9..ae0001e80840c2f6df9d773b107b05138bbe00b5 100644 (file)
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -434,24 +434,27 @@ static size_t ltdb_get_size(struct ldb_kv_private *ldb_kv)
 }
 
 static const struct kv_db_ops key_value_ops = {
-    .store = ltdb_store,
-    .delete = ltdb_delete,
-    .iterate = ltdb_traverse_fn,
-    .update_in_iterate = ltdb_update_in_iterate,
-    .fetch_and_parse = ltdb_parse_record,
-    .iterate_range = ltdb_iterate_range,
-    .lock_read = ltdb_lock_read,
-    .unlock_read = ltdb_unlock_read,
-    .begin_write = ltdb_transaction_start,
-    .prepare_write = ltdb_transaction_prepare_commit,
-    .finish_write = ltdb_transaction_commit,
-    .abort_write = ltdb_transaction_cancel,
-    .error = ltdb_error,
-    .errorstr = ltdb_errorstr,
-    .name = ltdb_name,
-    .has_changed = ltdb_changed,
-    .transaction_active = ltdb_transaction_active,
-    .get_size = ltdb_get_size,
+       /* No support for any additional features */
+       .options = 0,
+
+       .store = ltdb_store,
+       .delete = ltdb_delete,
+       .iterate = ltdb_traverse_fn,
+       .update_in_iterate = ltdb_update_in_iterate,
+       .fetch_and_parse = ltdb_parse_record,
+       .iterate_range = ltdb_iterate_range,
+       .lock_read = ltdb_lock_read,
+       .unlock_read = ltdb_unlock_read,
+       .begin_write = ltdb_transaction_start,
+       .prepare_write = ltdb_transaction_prepare_commit,
+       .finish_write = ltdb_transaction_commit,
+       .abort_write = ltdb_transaction_cancel,
+       .error = ltdb_error,
+       .errorstr = ltdb_errorstr,
+       .name = ltdb_name,
+       .has_changed = ltdb_changed,
+       .transaction_active = ltdb_transaction_active,
+       .get_size = ltdb_get_size,
 };
 
 /*