Bug 13025 - Continuous re-index possible
Summary: Continuous re-index possible
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.7.0rc5
Hardware: All All
: P5 enhancement (vote)
Target Milestone: 4.7
Assignee: Stefan Metzmacher
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-11 03:32 UTC by Andrew Bartlett
Modified: 2017-10-20 09:18 UTC (History)
2 users (show)

See Also:


Attachments
proposed patch for master (2.00 KB, patch)
2017-09-12 06:26 UTC, Andrew Bartlett
no flags Details
patch for 4.7 cherry-picked from master (6.37 KB, patch)
2017-09-16 06:43 UTC, Andrew Bartlett
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2017-09-11 03:32:02 UTC
The use of dsdb_replace() in dsdb_schema_set_indices_and_attributes() was corrected by ec9b1e881c3eef503d6b4b311594113acf7d47d8 but this did not consider removal of attributes.  It also did not assume a global read lock, which we now have.

The impact is that an attribute like @IDXGUID or @INDEXVERSION is not removed despite being detected as needing removal, causing a reindex on every db load.
Comment 1 Andrew Bartlett 2017-09-12 06:26:14 UTC
Created attachment 13580 [details]
proposed patch for master

Here is what I proposed for master.  I've tested this backported to 4.7.
Comment 2 Stefan Metzmacher 2017-09-12 08:31:05 UTC
Can you send the patch for master to the list?
Comment 4 Andrew Bartlett 2017-09-16 06:43:03 UTC
Created attachment 13604 [details]
patch for 4.7 cherry-picked from master

Sorry for the delay in proposing the backport before 4.7
Comment 5 Stefan Metzmacher 2017-09-17 16:58:19 UTC
I've just pushed to fix without the test with the review from slow@samba.org.

We can push the test later, but it's not critical for 4.7.0.

But we need to remove the knownfail hunk.
Comment 6 Stefan Metzmacher 2017-09-17 17:26:34 UTC
Pushed to autobuild-v4-7-test
Comment 7 Andrew Bartlett 2017-09-18 03:36:06 UTC
I did actually check it with the tests, but either way I'm sure we can sort the rest out after 4.7.
Comment 8 Stefan Metzmacher 2017-10-02 07:05:26 UTC
Test pushed to autobuild-v4-7-test
Comment 9 Stefan Metzmacher 2017-10-20 09:18:08 UTC
(In reply to Stefan Metzmacher from comment #8)

Pushed to v4-7-test