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

[Stefan Metzmacher]

Hi Bob,

> 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?

I'll have a look at this soon.

Would it be possible if you send patches as one text attachment in future
using git format-patch --stdout and using .txt at the end?
Otherwise thunderbird doesn't send it as text/plain.
See
https://lists.samba.org/archive/samba-technical/2017-February/118510.html
vs.
https://lists.samba.org/archive/samba-technical/2017-January/118096.html

I very often read mails via https://lists.samba.org/archive/samba-technical/
and it would be nice to be able to read the diffs there without downloading
files and open on editor for each file.

Thanks!
metze

> 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: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170202/3a3f6fbf/signature.sig>