[PATCH] Fix server side DRSUAPI_DRS_GET_ANC handling (bug #12398)

Wed Feb 1 01:32:06 UTC 2017

Hi Metze,

Attached is the proposed patchset, including 6 of your patches. Could
you please give your sign off for (and change the commit message of if
necessary) 0004-torture-drs-add-a-test-for-DRSUAPI_DRS_GET_ANC?

Thanks,
Bob

On 17/01/17 16:06, Bob Campbell wrote:
> I've placed our patches on top of 6 of Metze's, including the one
> indicated, and placed those onto master. I now see the issue with
> putting the ancestor collection in getncchanges_collect_objects(). The
> tests pass when all the patches are on. The tree is at
> http://git.catalyst.net.nz/gw?p=samba.git;a=shortlog;h=refs/heads/17-01-anc-patches.
>
> Metze: Should "expand tests" and your test commit be merged? Another way
> to do it would possibly be to move the tests into getnc_exop in a
> separate patch from adding my new test cases.
>
> Thanks,
> Bob
>
> On 12/01/17 22:05, Stefan Metzmacher wrote:
>> Am 12.01.2017 um 09:55 schrieb Andrew Bartlett:
>>> On Thu, 2017-01-12 at 09:44 +0100, Stefan Metzmacher wrote:
>>>> Am 12.01.2017 um 09:23 schrieb Andrew Bartlett:
>>>>> On Thu, 2016-12-15 at 12:04 +1300, Bob Campbell wrote:
>>>>>> We also understand the issue you've pointed out here. It seems
>>>>>> that
>>>>>> the
>>>>>> functionality we've put in getncchanges_collect_objects is split
>>>>>> into
>>>>>> two functions in Windows: GetReplScope (4.1.10.5.3) and
>>>>>> GetChangesInScope (4.1.10.5.5). In GetReplScope, they always add
>>>>>> ancestors of critical objects to the scope if
>>>>>> DRSUAPI_DRS_CRITICAL_ONLY
>>>>>> is set (even if DRSUAPI_DRS_GET_ANC isn't set) - what
>>>>>> documentation
>>>>>> do
>>>>>> you mean when you say that it's not how it's done in the
>>>>>> documentation?
>>>>>> We're going off of MS-DRSR. You are definitely correct in that
>>>>>> the
>>>>>> non-critical ancestors shouldn't impact the new_highwatermark,
>>>>>> and
>>>>>> indeed we possibly shouldn't change the new_highwatermark at all
>>>>>> if
>>>>>> DRSUAPI_DRS_CRITICAL_ONLY is set, as described in the related
>>>>>> problem.
>>>>> Metze,
>>>>>
>>>>> We have had a customer issue come up that looks to be related to
>>>>> this
>>>>> issue (incorrect GET_ANC handling).  Can you help us understand
>>>>> what is
>>>>> needed to make some progress on getting the work you and Bob have
>>>>> done
>>>>> into master?
>>>>>
>>>>> I agree that it is critical (for performance) that we not add the
>>>>> ancestors within getncchanges_collect_objects(), which needs to be
>>>>> absolutely as simple as possible, as it deals with the whole domain
>>>>> during a full replication.
>>>> I'd just take this patch
>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=8d156d088
>>>> 5e31ad6b80c1979efdfa7cfee075215
>>>> which works for ours customers just fine and do further improvements
>>>> on top of it.
>>> Which SerNet samba.plus RPMs include this patch? (my customers are also
>>> your customers ;-)
>>>
>>> I'll work with Bob tomorrow to understand better where his work and
>>> yours intersect, so we can move towards merging something with tests.  
>>>
>>> I'll see if Bob can re-work his patches to be on top of your patch. 
>>> Aside from where we add the ancestors, is there anything else major
>>> outstanding I should be aware of?
>> Starting with 4.5.3, it's not yet in 4.4 or something older (at least yet).
>>
>> metze
>>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-getncchanges-do-not-replicate-links-for-non-critical.patch
Type: text/x-patch
Size: 1993 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0010-getncchanges-do-not-replicate-links-for-non-critical-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-getncchanges-do-not-add-duplicate-objects-with-DRSUA.patch
Type: text/x-patch
Size: 2524 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0009-getncchanges-do-not-add-duplicate-objects-with-DRSUA-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-getncchanges-filter-ancestor-objects-by-uptodateness.patch
Type: text/x-patch
Size: 2253 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0008-getncchanges-filter-ancestor-objects-by-uptodateness-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-torture-drs-expand-test-for-DRSUAPI_DRS_GET_ANC.patch
Type: text/x-patch
Size: 43406 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0007-torture-drs-expand-test-for-DRSUAPI_DRS_GET_ANC-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-torture-drs-add-a-test-for-DRSUAPI_DRS_GET_ANC.patch
Type: text/x-patch
Size: 24675 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0006-torture-drs-add-a-test-for-DRSUAPI_DRS_GET_ANC-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-s4-rpc_server-drsuapi-don-t-process-DRSUAPI_DRS_CRIT.patch
Type: text/x-patch
Size: 1680 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0005-s4-rpc_server-drsuapi-don-t-process-DRSUAPI_DRS_CRIT-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-s4-rpc_server-drsuapi-only-set-nc_-object-linked_att.patch
Type: text/x-patch
Size: 1332 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0004-s4-rpc_server-drsuapi-only-set-nc_-object-linked_att-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-drsuapi.idl-add-drsuapi_DrsMoreOptions-with-DRSUAPI_.patch
Type: text/x-patch
Size: 1249 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0003-drsuapi.idl-add-drsuapi_DrsMoreOptions-with-DRSUAPI_-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-s4-rpc_server-drsuapi-remember-the-ncRoot_guid-on-th.patch
Type: text/x-patch
Size: 2657 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0002-s4-rpc_server-drsuapi-remember-the-ncRoot_guid-on-th-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s4-rpc_server-drsuapi-implement-DRSUAPI_DRS_GET_ANC-.patch
Type: text/x-patch
Size: 8475 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170201/62e0b27c/0001-s4-rpc_server-drsuapi-implement-DRSUAPI_DRS_GET_ANC--0001.bin>