#49376 Raise idlistscanlimit by default
Opened 2 years ago by firstyear. Modified 2 years ago

Issue Description

With https://pagure.io/389-ds-base/issue/49372 we try to avoid hitting large IDLs by fixing the filters.

There are some cases left we may access IDLs and in those cases, we probably want to return an IDL anyway.

One case is if we hit the idlistscanlimit in an OR query. In this case we return ALLIDS to the parent filter, which likely inturn raises this back to the parent. This means that we do a very expensive id2entry scan. Had we of return an IDL, even if the IDL had say 100,000 items, that's still better than doing a full id2entry on a db with 200,000.

Another case is broad AND queries. If we do a subtree search of objectClass=group under say ou=Groups, if this returns ALLIDS, we have just wasted a lot of time again falling back to a full db search when we could have loaded the IDL and intersected it with parentid.

I think that we should examine the current idlistscanlimit and either raise it, or consider removing it all together.


Here is an example:

idlistscanlimit: 4000 (default)
[11/Sep/2017:12:03:33.524447131 +1000] conn=2 op=1 SRCH base="dc=example,dc=com" scope=2 filter="(&(objectClass=inetOrgPerson)(objectClass=person)(objectClass=posixAccount)(uid>=test0053715))" attrs="uid"
[11/Sep/2017:12:03:54.236090743 +1000] conn=2 op=1 RESULT err=0 tag=101 nentries=46286 etime=20.1288196523 notes=A

idlistscanlimit: 99999999999
[11/Sep/2017:12:01:34.797114055 +1000] conn=4 op=1 SRCH base="dc=example,dc=com" scope=2 filter="(&(objectClass=inetOrgPerson)(objectClass=person)(objectClass=posixAccount)(uid>=test0053715))" attrs="uid"
[11/Sep/2017:12:01:45.866876775 +1000] conn=4 op=1 RESULT err=0 tag=101 nentries=46286 etime=11.0069939325

So despite being over 40,000 entries, using the indexes is still faster.

In fact, it looks like the idlistscan limit is an attempt to optimise the "(&(uid=x)(objectClass=y))" case to improve this search, but the recent changes to filter_optimise in https://pagure.io/389-ds-base/issue/49372 solve this much more completely.

I think we should remove the idlistscanlimit, as no other database (IE postgresql) has such an odd concept of bypassing indexes. We can gain more from improve filter optimise than trying to hand tune these numbers for specific cases,

Metadata Update from @firstyear:
- Custom field component adjusted to None
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Custom field type adjusted to None
- Custom field version adjusted to None

2 years ago

Following this, this would actually be a case to raise the filter test threshold, because this would allow better shortcutting of genuinely small sets that want to avoid large indexes. IE if we set test thrushold to say 32, but then if the set is larger than this, it's probably better to actually use the index. We could do some testing of the numbers here to be sure, but 32 - 64 could be good guesses.

Another point is that to maek filter optimisation easier, perhaps we should change their structure from linked list to something easier to sort / re-arrange?

Metadata Update from @firstyear:
- Issue assigned to firstyear

2 years ago

Metadata Update from @mreynolds:
- Issue set to the milestone: 1.4 backlog

2 years ago

Metadata Update from @firstyear:
- Custom field reviewstatus adjusted to review (was: None)

2 years ago

I don't think we should change the default.

There are some scenarios wher it might be better to always return an idl. It is always a decision what is more costly: to handle and manage a very large idlist (it also has a lot of database accesses) or to handle all entries (which might be already in the entry cache).
The scanlimit is configurable and GSS knows to suggest proper settings.
If we change index lookup and idl handling (in other tickets) we need to document the effects it might have an the use of scan limits.
If we are really confident that always returning and idl would be the right thing, then we should remove the scan limit at all.

In general I think for changes like this we should have a set of reliable performance benchmarks first.

@lkrispen I have been performing a number of tests with the filter optimisation patch enabled, and in all cases the idlistscanlimit of 999999999 exceeded the performance of the current default. I will prepare a set of numbers for you.

As much as I love and respect GSS, they do NOT know how to suggest a proper setting here. This is a deeply confusing setting that relies on intimate knowledge of our internal search procedure that most users don't have.

I agree that we should remove the scan limit completely, but I would like to do this first to give us a clean backout strategy if it does fail on us,

waiting for the set of numbers

Yep! I need to make the numbers for you! I've been quite busy as you might know, and I want to work on this with @vashirov to make our performance test framework? If you are going to do something, may as well do it correctly.

Login to comment on this ticket.

Metadata
Attachments 1