Skip to content

[autobackport: sssd-2-9] memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop()#8559

Merged
alexey-tikhonov merged 1 commit intoSSSD:sssd-2-9from
sssd-bot:SSSD-sssd-backport-pr8546-to-sssd-2-9
Apr 1, 2026
Merged

[autobackport: sssd-2-9] memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop()#8559
alexey-tikhonov merged 1 commit intoSSSD:sssd-2-9from
sssd-bot:SSSD-sssd-backport-pr8546-to-sssd-2-9

Conversation

@sssd-bot
Copy link
Copy Markdown
Contributor

This is an automatic backport of PR#8546 memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop() to branch sssd-2-9, created by @alexey-tikhonov.

Please make sure this backport is correct.

Note

The commits were cherry-picked without conflicts.

You can push changes to this pull request

git remote add sssd-bot git@github.com:sssd-bot/sssd.git
git fetch sssd-bot refs/heads/SSSD-sssd-backport-pr8546-to-sssd-2-9
git checkout SSSD-sssd-backport-pr8546-to-sssd-2-9
git push sssd-bot SSSD-sssd-backport-pr8546-to-sssd-2-9 --force

Original commits
2dcdca2 - memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop()

Backported commits

  • 87e0256 - memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop()

Original Pull Request Body

Justification is the same as in 704c31d
In certain scenarios this function is a hot path and using heavy ldb_dn_compare() adds unnecessary overhead.

Testing with a following setup: 50 mid-level groups that all share the same 2500 leaf groups as members, plus one top-level group containing all mid-level groups. Each leaf group contains a single user. ignore_group_members = false.
time id user with a cold cache:

i.e. this patch makes mbof_append_addop() approx x2 faster (this gain doesn't depend on number of groups; but weight of mbof_append_addop() in total consumption depends on it)

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request moves the sss_linearized_dn_match function to an earlier position in src/ldb_modules/memberof.c and updates mbof_append_addop to use it for DN comparisons instead of ldb_dn_compare. It also introduces a null check for the linearized DN to improve robustness. I have no feedback to provide.

@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Mar 28, 2026
@alexey-tikhonov alexey-tikhonov removed the request for review from justin-stephenson March 30, 2026 07:09
Copy link
Copy Markdown
Contributor

@aplopez aplopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Justification is the same as in 704c31d
In certain scenarios this function is a hot path and using heavy
`ldb_dn_compare()` adds unnecessary overhead.

Reviewed-by: Alejandro López <allopez@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
(cherry picked from commit 2dcdca2)
@sssd-bot
Copy link
Copy Markdown
Contributor Author

sssd-bot commented Apr 1, 2026

The pull request was accepted by @alexey-tikhonov with the following PR CI status:


🟢 CodeQL (success)
🟢 rpm-build:centos-stream-9-x86_64:upstream (success)
🟢 Build / make-distcheck (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-9) (success)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@sssd-bot sssd-bot force-pushed the SSSD-sssd-backport-pr8546-to-sssd-2-9 branch from 87e0256 to 4c7abf9 Compare April 1, 2026 09:22
@alexey-tikhonov alexey-tikhonov merged commit 01b2bd3 into SSSD:sssd-2-9 Apr 1, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted no-backport This should go to target branch only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants