SDAP: reduce logger load in the hot paths#8540
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 refactors logging within the sdap_get_generic_ext_step function to reduce performance overhead in this hot path. The changes include consolidating multiple DEBUG calls into a single statement and using DEBUG_CONDITIONAL for logging requested attributes. This prevents populating the backtrace with attribute information unless a specific debug level is enabled. The changes are consistent with the goal of reducing logger load. I have reviewed the implementation and found no issues.
alexey-tikhonov
changed the title
|
Note: Covscan is green. |
|
Users : 12006 (1 user is a member of all groups) (I)
(II)
|
sumit-bose
left a comment
sumit-bose
left a comment
There was a problem hiding this comment.
Hi,
thank you for the patch. I agree that is is acceptable to loose the messages which are now send with DEBUG_CONDITIONAL from the backtrace, ACK.
bye,
Sumit
This patch reduced number of *sprintf() keeping the same level of details in the resulting log. Besides, list of attrs being requested was excluded from the backtrace. Reviewed-by: Iker Pedrosa <[email protected]> Reviewed-by: Sumit Bose <[email protected]>
Reviewed-by: Iker Pedrosa <[email protected]> Reviewed-by: Sumit Bose <[email protected]>
662d7d2 to
7374851
Compare
I got confirmation that this is simply a log change issue. We can fix the tests to check the new pattern. SSSD#8540 Reviewed-by: Scott Poore <[email protected]>
I got confirmation that this is simply a log change issue. We can fix the tests to check the new pattern. #8540 Reviewed-by: Scott Poore <[email protected]>
I got confirmation that this is simply a log change issue. We can fix the tests to check the new pattern. SSSD#8540 Reviewed-by: Scott Poore <[email protected]> (cherry picked from commit 04d5937)
I got confirmation that this is simply a log change issue. We can fix the tests to check the new pattern. #8540 Reviewed-by: Scott Poore <[email protected]> (cherry picked from commit 04d5937)
I got confirmation that this is simply a log change issue. We can fix the tests to check the new pattern. SSSD#8540 Reviewed-by: Scott Poore <[email protected]> (cherry picked from commit 04d5937)
I got confirmation that this is simply a log change issue. We can fix the tests to check the new pattern. #8540 Reviewed-by: Scott Poore <[email protected]> (cherry picked from commit 04d5937)
4 participants
Logger usage in
sdap_get_generic_ext_step()accounts for ~2..3% in the testwhen
ignore_group_members = falseand there are 12k users in total.Other logs changes contribute ~1% in the same test.