SBUS: increase SBUS_MESSAGE_TIMEOUT to 5 mins#8367
Conversation
alexey-tikhonov
added
backport-to-sssd-2-9
backport-to-sssd-2-9-4
![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 increases the SBUS message timeout to 5 minutes to accommodate long-running operations and also fixes a bug in the client idle timer logic. The changes to the timer logic appear correct. However, a new check introduced to compare the client idle timeout with the SBUS message timeout contains a bug due to a unit mismatch (seconds vs. milliseconds), which I've commented on.
3e4a257 to
9a5d3c2
Compare
| if (rctx->client_idle_timeout > SBUS_MESSAGE_TIMEOUT/1000) { | ||
| DEBUG(SSSDBG_CONF_SETTINGS, "'"CONFDB_RESPONDER_CLI_IDLE_TIMEOUT | ||
| "' is configured to a value %d - higher than SBUS_MESSAGE_TIMEOUT %d, " | ||
| "this usually doesn't make sense.\n", |
There was a problem hiding this comment.
Hi,
if you add a comment here, it might be worth to add a similar comment to the client_idle_timeout man page entry as well.
bye,
Sumit
There was a problem hiding this comment.
Maybe it's better to remove this warning...
'client_idle_timeout' > 'SBUS_MESSAGE_TIMEOUT' can be justified actually.
It can be used to keep fd open between client requests...
There was a problem hiding this comment.
I'm fine with removing it.
9a5d3c2 to
b67a1d6
Compare
alexey-tikhonov
added
coverity
|
Note: Covscan is green. |
b67a1d6 to
1ce72a7
Compare
sumit-bose
left a comment
sumit-bose
left a comment
There was a problem hiding this comment.
Hi,
thank you, I have no further comments, ACK.
bye,
Sumit
alexey-tikhonov
added
backport-to-sssd-2-11
Accepted
and removed
backport-to-sssd-2-11
Waiting for review
labels
Handling BE_REQ_INITGROUPS for LDAP user with 10k groups takes longer than 2 mins. Reviewed-by: Pavel Březina <[email protected]> Reviewed-by: Sumit Bose <[email protected]>
As it was implemented previously, effective period was 1.5*client_idle_timeout instead of `client_idle_timeout` as documented. Log with default value - 60 - before a fix: ``` (:49:12): [nss] [setup_client_idle_timer] (0x4000): Idle timer re-set for client [0x557af16f31b0][22] (:49:42): [nss] [setup_client_idle_timer] (0x4000): Idle timer re-set for client [0x557af16f31b0][22] (:50:12): [nss] [setup_client_idle_timer] (0x4000): Idle timer re-set for client [0x557af16f31b0][22] (:50:42): [nss] [client_idle_handler] (0x2000): Terminating idle client [0x557af16f31b0][22] ``` Reviewed-by: Pavel Březina <[email protected]> Reviewed-by: Sumit Bose <[email protected]>
1ce72a7 to
c8e14d0
Compare
4 participants
... and fixed an issue with 'client_idle_timer'