[PATCH] Final Fix for DNS wildcard performance regression
Andrew Bartlett
abartlet at samba.org
Mon Dec 18 04:19:24 UTC 2017
On Mon, 2017-12-18 at 13:21 +1300, Andrew Bartlett wrote:
> The attached patches for master partially address the DNS performance
> regression found in Samba 4.7.3.
I've looked into this some more, and the attached patches finish the
job and return the performance to the original behaviour.
These are for master, a backport to 4.7 I'll try once it lands. In the
meantime the patch posted earlier will help a lot.
Please review and push!
Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
-------------- next part --------------
From 3d6d02706c660f6706d8fdd3da7098f0854efba7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 18 Dec 2017 16:22:01 +1300
Subject: [PATCH 1/4] ldb: Intersect the index from SCOPE_ONELEVEL with the
index for the search expression
This helps ensure we do not have to scan all objects at this level
which could be very many (one per DNS zone entry).
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb/ldb_tdb/ldb_index.c | 51 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 0afeae57a60..d79851ea52f 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -151,6 +151,13 @@ ldb_schema_set_override_GUID_index() must be called.
struct dn_list {
unsigned int count;
struct ldb_val *dn;
+ /*
+ * Do not optimise the intersection of this list,
+ * we must never return an entry not in this
+ * list. This allows the index for
+ * SCOPE_ONELEVEL to be trusted.
+ */
+ bool strict;
};
struct ltdb_idxptr {
@@ -1029,10 +1036,10 @@ static bool list_intersect(struct ldb_context *ldb,
what really matches, as all results are filtered by the
full expression at the end - this shortcut avoids a lot of
work in some cases */
- if (list->count < 2 && list2->count > 10) {
+ if (list->count < 2 && list2->count > 10 && list2->strict == false) {
return true;
}
- if (list2->count < 2 && list->count > 10) {
+ if (list2->count < 2 && list->count > 10 && list->strict == false) {
list->count = list2->count;
list->dn = list2->dn;
/* note that list2 may not be the parent of list2->dn,
@@ -1073,6 +1080,7 @@ static bool list_intersect(struct ldb_context *ldb,
}
}
+ list->strict |= list2->strict;
list->dn = talloc_steal(list, list3->dn);
list->count = list3->count;
talloc_free(list3);
@@ -1411,6 +1419,8 @@ static int ltdb_index_dn_one(struct ldb_module *module,
struct ldb_dn *parent_dn,
struct dn_list *list)
{
+ /* Ensure we do not shortcut on intersection for this list */
+ list->strict = true;
return ltdb_index_dn_attr(module, ltdb,
LTDB_IDXONE, parent_dn, list);
}
@@ -1630,6 +1640,7 @@ static void ltdb_dn_list_sort(struct ltdb_private *ltdb,
*/
int ltdb_search_indexed(struct ltdb_context *ac, uint32_t *match_count)
{
+ struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
struct ltdb_private *ltdb = talloc_get_type(ldb_module_get_private(ac->module), struct ltdb_private);
struct dn_list *dn_list;
int ret;
@@ -1677,8 +1688,42 @@ int ltdb_search_indexed(struct ltdb_context *ac, uint32_t *match_count)
talloc_free(dn_list);
return ret;
}
- break;
+ /*
+ * If we have too many matches, also try the filter
+ * tree and do index work there
+ */
+ if (dn_list->count > 10) {
+ struct dn_list *idx_one_tree_list
+ = talloc_zero(ac, struct dn_list);
+ if (idx_one_tree_list == NULL) {
+ return ldb_module_oom(ac->module);
+ }
+
+ if (!ltdb->cache->attribute_indexes) {
+ talloc_free(idx_one_tree_list);
+ talloc_free(dn_list);
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ /*
+ * Here we load the index for the tree.
+ */
+ ret = ltdb_index_dn(ac->module, ltdb, ac->tree,
+ idx_one_tree_list);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(idx_one_tree_list);
+ talloc_free(dn_list);
+ return ret;
+ }
+
+ if (!list_intersect(ldb, ltdb,
+ dn_list, idx_one_tree_list)) {
+ talloc_free(idx_one_tree_list);
+ talloc_free(dn_list);
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ }
+ break;
case LDB_SCOPE_SUBTREE:
case LDB_SCOPE_DEFAULT:
if (!ltdb->cache->attribute_indexes) {
--
2.11.0
From 98ac2c4a54e2d5131f2c026b21d79cff7fadd493 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 18 Dec 2017 16:22:23 +1300
Subject: [PATCH 2/4] dns_server: Use the indexed "name" attribute in wildcard
lookup
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dns_server/dnsserver_common.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index 217e65b39f4..a6fe4804130 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -305,6 +305,8 @@ static unsigned int number_of_labels(const struct ldb_val *name) {
*
* x.y.z -> (|(name=x.y.z)(name=\2a.y.z)(name=\2a.z)(name=\2a))
*
+ * The attribute 'name' is used as this is what the LDB index is on
+ *
* Returns NULL if unable to build the query.
*
* The first component of the DN is assumed to be the name being looked up
@@ -318,7 +320,7 @@ static struct ldb_parse_tree *build_wildcard_query(
{
const struct ldb_val *name = NULL; /* The DNS name being
queried */
- const char *attr = NULL; /* The attribute name */
+ const char *attr = "name"; /* The attribute name */
struct ldb_parse_tree *query = NULL; /* The constructed query
parse tree*/
struct ldb_parse_tree *wildcard_query = NULL; /* The parse tree for the
@@ -326,12 +328,6 @@ static struct ldb_parse_tree *build_wildcard_query(
entries */
int labels = 0; /* The number of labels in the name */
- attr = ldb_dn_get_rdn_name(dn);
- if (attr == NULL) {
- DBG_ERR("Unable to get rdn_name\n");
- return NULL;
- }
-
name = ldb_dn_get_rdn_val(dn);
if (name == NULL) {
DBG_ERR("Unable to get domain name value\n");
--
2.11.0
From 5287fd2eb0d990a1b4869388933e753f79f72cc2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 15 Dec 2017 11:40:28 +1300
Subject: [PATCH 3/4] dns_server: Do not look for a wildcard for @
This query is made for every record returned via BIND9 DLZ.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13191
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dns_server/dnsserver_common.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index a6fe4804130..b2b2d1f4e1a 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -543,6 +543,16 @@ WERROR dns_common_wildcard_lookup(struct ldb_context *samdb,
return DNS_ERR(NAME_ERROR);
}
+ /* Don't look for a wildcard for @ */
+ if (name->length == 1 && name->data[0] == '@') {
+ return dns_common_lookup(samdb,
+ mem_ctx,
+ dn,
+ records,
+ num_records,
+ NULL);
+ }
+
werr = dns_name_check(
mem_ctx,
strlen((const char*)name->data),
--
2.11.0
From 2480d5e91c9ce3c1729282cbe9c8c77b59403692 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 15 Dec 2017 12:30:50 +1300
Subject: [PATCH 4/4] dns_server: Do the exact match query first, then do the
wildcard lookup
The wildcard lookup is SCOPE_ONELEVEL which in current ldb will load the
full index for that zone.
This is inefficient as we do not currently intersect that index with the
index on DN=
Not that a not-found and wildcard response will still go via the ONELEVEL
index for now so this only helps for correct, directly matching queries.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13191
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dns_server/dnsserver_common.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index b2b2d1f4e1a..38acfff9a00 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -561,6 +561,20 @@ WERROR dns_common_wildcard_lookup(struct ldb_context *samdb,
return werr;
}
+ /*
+ * Do a point search first, then fall back to a wildcard
+ * lookup if it does not exist
+ */
+ werr = dns_common_lookup(samdb,
+ mem_ctx,
+ dn,
+ records,
+ num_records,
+ NULL);
+ if (!W_ERROR_EQUAL(werr, WERR_DNS_ERROR_NAME_DOES_NOT_EXIST)) {
+ return werr;
+ }
+
ret = dns_wildcard_lookup(samdb, mem_ctx, dn, &msg);
if (ret == LDB_ERR_OPERATIONS_ERROR) {
return DNS_ERR(SERVER_FAILURE);
--
2.11.0
More information about the samba-technical
mailing list