Improve the performance when using enumeration#8395
Improve the performance when using enumeration#8395aplopez wants to merge 6 commits intoSSSD:masterfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively improves performance by optimizing logging, removing an unused function, and correcting a condition related to enumeration. The changes are well-aligned with the stated goals of enhancing enumeration performance, especially for large user bases. The addition of a new test case for the general enumeration scenario ensures that the modified logic is adequately covered.
|
Mistype in the commit message: "We must look into de TS cache" |
Fixed. |
|
I think fix is correct in the sense it fixes a bug. But I think logic of In particular, if |
Function sysdb_enumpwent() is not used. It was replaced by sysdb_enumpwent_filter().
When there are too many users (17,000+) this message can be too long. Limit it to the first 50 characters. Resolves: SSSD#6951
We must look into the TS cache only when a name is provided. Using the TS cache on an unfiltered enumeration is useless. Resolves: SSSD#6951
Added a case that was not checked before. It is the case when `attr`, `attr_name` and `addtl_filter` are all `NULL`.
Create the filter to retrieve only the requested entries. Do not create a new filter and search for matches if there is no results from the previous search. The called functions handle this case correctly but why waisting time calling them?
Function cache_req_user_by_filter_lookup() will set or not the recent filter depending on whether data->name.attr is set or not. As mentioned in the comment, it should be done base on whether the refernced attribute is name or not.
| dn_filter, &ts_cache_res); | ||
| if (ret != EOK && ret != ENOENT) { | ||
| goto done; | ||
| } |
There was a problem hiding this comment.
Can this go out immediately from else branch?
There was a problem hiding this comment.
if (ts_res.count > 0) {} else {go-out}
3 participants
This PR includes:
Enumeration, specially when there are 15,000+ users, is slow. This fix helps, but it doesn't work miracles.
In my test environment, the enumeration went from 8 minutes to about 1.
It is important to know that, with such an amount of users, many operations time out. It is necessary to increment the
timeoutin[nss]and for the domain, but also set large values forldap_enumeration_refresh_timeoutandldap_search_timeoutin the domain. I used these values to avoid any timeout (YMMV):