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.
alexey-tikhonov
force-pushed
the
sbus-timeout
branch
from
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.
alexey-tikhonov
force-pushed
the
sbus-timeout
branch
from
9a5d3c2 to
b67a1d6
Compare
alexey-tikhonov
added
coverity
|
Note: Covscan is green. |
alexey-tikhonov
force-pushed
the
sbus-timeout
branch
from
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 <pbrezina@redhat.com> Reviewed-by: Sumit Bose <sbose@redhat.com>
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 <pbrezina@redhat.com> Reviewed-by: Sumit Bose <sbose@redhat.com>
sssd-bot
force-pushed
the
sbus-timeout
branch
from
1ce72a7 to
c8e14d0
Compare
4 participants
... and fixed an issue with 'client_idle_timer'