Bug 54092 - Add LDAP eq Index for krb5ValidStart
Add LDAP eq Index for krb5ValidStart
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: LDAP
UCS 5.0
Other Linux
: P5 normal (vote)
: UCS 5.0-1-errata
Assigned To: Alexander Steffen
Daniel Tröder
https://help.univention.com/t/problem...
:
Depends on: 53631
Blocks:
  Show dependency treegraph
 
Reported: 2021-11-18 16:02 CET by Arvid Requate
Modified: 2022-08-08 13:39 CEST (History)
5 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): UCS Performance
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate univentionstaff 2021-11-18 16:02:39 CET
The standard feature developed for Bug #53631 uses a cron job which periodically searches for "(krb5ValidStart<=$now)". The attribute should be added to the eq index.

The discussion at https://www.openldap.com/lists/openldap-technical/201205/msg00027.html doesn't recommend to index for combined filters like "(&(krb5ValidStart<=$now)(disabled=1)", but in this case the script

/usr/share/univention-directory-manager-tools/univention-delayed-account-activation

doesn't use a combined filter and the script removes that attribute for all matched accounts, so the number of match candidates should always be low, so an index should be beneficial.
Comment 1 Philipp Hahn univentionstaff 2021-11-18 16:26:54 CET
Just to satisfy my hunger for knowledge: Does OpenLDAP has a "EXPLAIN" to investigate its query plan?

I'm asking this as I haven't found any information on if OpenLDAP does query optimization by itself, e.g. if swapping the constraints makes any differences:

1. (&(krb5ValidStart<=$now)(disabled=1))
2. (&(disabled=1)(krb5ValidStart<=$now))

- Lacking any index a full scan has to be performed anyway -> slow

FOR entry IN ALL:
  IF NOT (entry.krb5ValidStart <= $now): CONTINUE
  IF NOT (entry.disabled == 1): CONTINUE
  YIELD entry

- If you have an index you normally can drastically reduce the number of entries to check
  - if the other constrains do not have an index you just iterate over them and check them directly

FOR entry IN FROM_BTREE_INDEX(SEEK="krb5ValidStart = $now", DIRECTION=backwards):
  IF NOT (entry.disabled == 1): CONTINUE
  YIELD entry

FOR entry IN FROM_HASHED_INDEX("disabled == 1"):
  IF NOT (entry.krb5ValidStart <= $now): CONTINUE
  YIELD entry

  - if other constrains are also indexed, you can combine both index results via logical AND or OR. This might be very expensive if your number of potential objects is large as then you have to create, combine, free large bitmaps.

BY_krb5ValidStart := {entry FOR entry IN FROM_BTREE_INDEX(SEEK="krb5ValidStart = $now", DIRECTION=backwards) 
BY_valid := {entry FOR entry IN FROM_HASHED_INDEX("disabled == 1")
FOR entry IN (BY_krb5ValidStart & BY_valid):
  YIELD entry

That questions was also asked at <https://www.openldap.org/lists/openldap-technical/201204/msg00124.html> but remains unanswered there.
Comment 3 Alexander Steffen univentionstaff 2021-12-17 10:00:35 CET
Issue: 
krb5ValidStart was not added to eq-index (LDAP) in any way.

Fix:
Added attribute (static) within recommended indexlist at file "ldap_setup_index"


fixed with branch: asteffen/54092-add-krb5VailidStart-to-eq-index

requesting QA
Comment 4 Daniel Tröder univentionstaff 2022-01-10 15:53:54 CET
To see the problem, execute
# /usr/share/univention-directory-manager-tools/univention-delayed-account-activation
# grep krb5ValidStart /var/log/syslog | tail
Comment 5 Daniel Tröder univentionstaff 2022-01-10 16:16:02 CET
After installing the updated package, the problem remains.
The script "ldap_setup_index" was not executed by the update.
Comment 6 Michael Grandjean univentionstaff 2022-01-11 17:15:10 CET
Please don't invoke slapindex via an errata update. This will cause severe downtimes on bigger installations and most maintenance windows for errata updates in the wild are not long enough for this.

Maybe just run it on new installs and add a hint to the changelog and UCS 5.0-2 release notes to run slapindex/ldap_setup_index manually on existing installs
Comment 7 Arvid Requate univentionstaff 2022-01-11 22:41:40 CET
Also, for new installs an explicit reindex doesn't make sense, does it?
Comment 8 Daniel Tröder univentionstaff 2022-01-13 18:03:23 CET
OK, please merge the change to 5.0-1, changelog, build and create an advisory (yaml) that contains instructions how to manually run the script, so that it creates the index. Alternatively point in the yaml to an existing page on help.univention.com or create a new one, that explains the steps.
Comment 9 Alexander Steffen univentionstaff 2022-01-21 10:41:06 CET
rebased branch to avoid copyright errors in pipelines.

mergerequest can be found here:

https://git.knut.univention.de/univention/ucs/-/merge_requests/254
Comment 10 Alexander Steffen univentionstaff 2022-01-25 08:57:04 CET
changes haven been applied with

https://git.knut.univention.de/univention/ucs/-/commit/92dccc0b2e36cc157058c09a60aa172422451673

and merged with 5.0-1
Comment 11 Alexander Steffen univentionstaff 2022-01-25 08:57:33 CET
changes haven been applied with

https://git.knut.univention.de/univention/ucs/-/commit/92dccc0b2e36cc157058c09a60aa172422451673

and merged with 5.0-1
Comment 12 Philipp Hahn univentionstaff 2022-01-27 12:12:11 CET
(In reply to Alexander Steffen from comment #11)
> changes haven been applied with
> 
> https://git.knut.univention.de/univention/ucs/-/commit/
> 92dccc0b2e36cc157058c09a60aa172422451673
> 
> and merged with 5.0-1

@asteffen Your added `Alexander Steffen <steffen@univention.de>  Tue, 25 Feb 2022 11:22:07 +0100` to `management/univention-ldap/debian/changelog`: date from the *future* now breaks `ucslint` everywhere.

PLEASE: rebase and squash your commits before merging in to main: probably nobody is interested in all the side-ways you had to take to achieve your goal, but a clean history helps everyone to to understand the change in the years to come.
Now we have a mess!
Comment 14 Alexander Steffen univentionstaff 2022-02-08 12:53:19 CET
Successful build
Package: univention-ldap
Version: 16.0.7-15A~5.0.0.202202081250
Branch: ucs_5.0-0
Scope: errata5.0-1
Comment 15 Daniel Tröder univentionstaff 2022-02-08 12:55:02 CET
OK: code change
OK: no automatic rebuild of indices
OK: help article explaining manual rebuild of indices
OK: index exists after executing code from help article
Comment 17 Arvid Requate univentionstaff 2022-08-08 13:39:02 CEST
I've added a KCS article to collect the current knowledge about this:

https://help.univention.com/t/problem-mdb-equality-candidates-exampleldapattribute-not-indexed