[PATCH] Partial fix for DNS performance regression

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Partial fix for DNS performance regression

Samba - samba-technical mailing list
The attached patches for master partially address the DNS performance
regression found in Samba 4.7.3.

In master, a DNS query against a large domain (7000 entries in the
zone) can take 18 seconds, much longer than the retry timeout (causing
back-off, retry, much pain).

Master (for 4.8) is already better than 4.7.3 in this area ('only'
10s), but these patches avoid the SCOPE_SUBTREE search until the
wildcard lookup is needed.

The patches for 4.7 include a backport of some already-made
improvements in Samba's master branch, part of the preparation patches
for the GUID index mode.

My rough notes on performance show that the LDB changes save about
another 20% for NXDOMAIN results, the DNS changes are the source of the
primary gains.

For the domain root, 'host domain.com' takes about 100ms in all cases
after the patch, returning to the baseline seen with 4.7.0.

However 'host foo.domain.com' takes:

(rough figures)
 - master:                   270ms
 - master + patch:           150ms
 - 4.7.3 (unpatched):       1000ms
 - 4.7.3 + patch:            340ms
 - 4.7.3 + DNS changes only: 500ms

 - 4.7.0:                     20ms

We are still an order of magnitude slower with wildcard DNS support
compared to before we added it, for NXDOMAIN responses.  This isn't OK,
so we need to keep working on this, but the attached should help for
now.

Please review carefully and push the patches to master, then we will
backport, perhaps for 4.7.4 if we are lucky.

Thanks,

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




dns-wildcard-perf-partial-4.7-fix.patch.txt (75K) Download Attachment
dns-wildcard-perf-partial-fix.patch.txt (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Partial fix for DNS performance regression

Samba - samba-technical mailing list
Seeing that the performance hit continues to be non-trivial, we should
probably include an option to simply turn wildcard off. For anyone
actually using wildcard, they will have to enable it (and incur the
performance hit). But for anyone else, is there enough justification for
having it (considering most people up until now haven't seemed to
required it)? 


Cheers,

Garming

On 18/12/17 13:21, Andrew Bartlett via samba-technical wrote:

> The attached patches for master partially address the DNS performance
> regression found in Samba 4.7.3.
>
> In master, a DNS query against a large domain (7000 entries in the
> zone) can take 18 seconds, much longer than the retry timeout (causing
> back-off, retry, much pain).
>
> Master (for 4.8) is already better than 4.7.3 in this area ('only'
> 10s), but these patches avoid the SCOPE_SUBTREE search until the
> wildcard lookup is needed.
>
> The patches for 4.7 include a backport of some already-made
> improvements in Samba's master branch, part of the preparation patches
> for the GUID index mode.
>
> My rough notes on performance show that the LDB changes save about
> another 20% for NXDOMAIN results, the DNS changes are the source of the
> primary gains.
>
> For the domain root, 'host domain.com' takes about 100ms in all cases
> after the patch, returning to the baseline seen with 4.7.0.
>
> However 'host foo.domain.com' takes:
>
> (rough figures)
>  - master:                   270ms
>  - master + patch:           150ms
>  - 4.7.3 (unpatched):       1000ms
>  - 4.7.3 + patch:            340ms
>  - 4.7.3 + DNS changes only: 500ms
>
>  - 4.7.0:                     20ms
>
> We are still an order of magnitude slower with wildcard DNS support
> compared to before we added it, for NXDOMAIN responses.  This isn't OK,
> so we need to keep working on this, but the attached should help for
> now.
>
> Please review carefully and push the patches to master, then we will
> backport, perhaps for 4.7.4 if we are lucky.
>
> Thanks,
>
> Andrew Bartlett


Reply | Threaded
Open this post in threaded view
|

[PATCH] Final Fix for DNS wildcard performance regression

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
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




dns-ldb-perf-fixes.patch.txt (11K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Final Fix for DNS wildcard performance regression

Samba - samba-technical mailing list
On Mon, 2017-12-18 at 17:19 +1300, Andrew Bartlett via samba-technical
wrote:

> 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!
Now with more BUG tags.

Thanks,

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




dns-ldb-perf-fixes.patch.txt (11K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Final Fix for DNS wildcard performance regression

Samba - samba-technical mailing list
On Mon, 2017-12-18 at 17:32 +1300, Andrew Bartlett wrote:

> On Mon, 2017-12-18 at 17:19 +1300, Andrew Bartlett via samba-technical
> wrote:
> > 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!
>
> Now with more BUG tags.
Attached is the final version.  Once that is in we can backport the
variant (attached) to 4.7.  

I need this in master first however to get commit hashes.

Please review and push!

Thanks,

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




dns-ldb-perf-fixes.patch.txt (11K) Download Attachment
dns-wildcard-perf-fix-4.7.patch.txt (67K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Final Fix for DNS wildcard performance regression

Samba - samba-technical mailing list
I think you need to comment on why using 'name' is important somewhere
in the patches (as opposed to the alternatives), considering it's the
most important part of the fix.

Apart from that, you can mark them as reviewed by me.


Cheers,

Garming


On 19/12/17 12:21, Andrew Bartlett wrote:

> On Mon, 2017-12-18 at 17:32 +1300, Andrew Bartlett wrote:
>> On Mon, 2017-12-18 at 17:19 +1300, Andrew Bartlett via samba-technical
>> wrote:
>>> 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!
>> Now with more BUG tags.
> Attached is the final version.  Once that is in we can backport the
> variant (attached) to 4.7.  
>
> I need this in master first however to get commit hashes.
>
> Please review and push!
>
> Thanks,
>
> Andrew Bartlett


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Final Fix for DNS wildcard performance regression

Samba - samba-technical mailing list
On Tue, 2017-12-19 at 13:09 +1300, Garming Sam wrote:
> I think you need to comment on why using 'name' is important somewhere
> in the patches (as opposed to the alternatives), considering it's the
> most important part of the fix.
>
> Apart from that, you can mark them as reviewed by me.
>

Sadly the patch fails make test, because it changes the semantics for

scope: SCOPE_ONELEVEL
expression (dn=dc=foo,dc=com)

This is illegal in AD (need to use distinguisedName), and returns 0
results with SCOPE_SUBTREE.  However it now returns 0 results for
SCOPE_ONELEVEL now, where it used to return 1.

Clearly I need a specific testsuite to cover this, as it was found by a
random unrelated testsuite.

I'll continue to work on this tomorrow.  

I would say 'hold the release' (as the AD DC is really quite unuseable
for domains with a lot of DNS entries) but I've thought a full fix was
a day away all week.  

The DNS server changes could go in, and would help a lot (I'll
autobuild those now).

Thanks,

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





Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Final Fix for DNS wildcard performance regression

Samba - samba-technical mailing list
On Tue, 2017-12-19 at 17:00 +1300, Andrew Bartlett via samba-technical
wrote:

> On Tue, 2017-12-19 at 13:09 +1300, Garming Sam wrote:
> > I think you need to comment on why using 'name' is important somewhere
> > in the patches (as opposed to the alternatives), considering it's the
> > most important part of the fix.
> >
> > Apart from that, you can mark them as reviewed by me.
> >
>
> Sadly the patch fails make test, because it changes the semantics for
>
> scope: SCOPE_ONELEVEL
> expression (dn=dc=foo,dc=com)
>
> This is illegal in AD (need to use distinguisedName), and returns 0
> results with SCOPE_SUBTREE.  However it now returns 0 results for
> SCOPE_ONELEVEL now, where it used to return 1.
>
> Clearly I need a specific testsuite to cover this, as it was found by a
> random unrelated testsuite.
I've added these tests.  The attached is in autobuild.

> I'll continue to work on this tomorrow.  
>
> I would say 'hold the release' (as the AD DC is really quite unuseable
> for domains with a lot of DNS entries) but I've thought a full fix was
> a day away all week.  

I plan to finish that shortly.

> The DNS server changes could go in, and would help a lot (I'll
> autobuild those now).

I've asked Garming to upload the DNS server changes with cherry-pick
tags.

Thanks,

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




ldb-onelevel-index-intersect.patch.txt (18K) Download Attachment