v4: Use a thread specific trunk for LDAP bind auths #4974
Conversation
src/lib/server/trunk.c
Outdated
| @@ -3167,7 +3167,7 @@ static void trunk_connection_enter_active(fr_trunk_connection_t *tconn) | |||
| /* | |||
| * Rebalance requests | |||
| */ | |||
| trunk_rebalance(trunk); | |||
| if (!trunk->conf.block_rebalance) trunk_rebalance(trunk); | |||
There was a problem hiding this comment.
It's not enough.. if the connection closed then the request would still get moved to another connection. It's probably be easier to remove the rebalance and to rework the code so that balancing was done in fr_trunk_connection_pop_request instead. The difficulty is how to actually do that balancing. I guess we could just allow as many requests to be popped as the max active request limit for the connection? I think it's 255 for RADIUS because we reserve one... one request per connection for "WAIT"ing redis... and I suppose as many requests it takes to fill the write buffer for LDAP. There the muxer would be self-limiting and stop popping requests once the write buffer was filled.
There was a problem hiding this comment.
Further testing has shown that per_connection_max is doing what is needed, so this change is not required. Not sure why it didn't work first time through - probably something to do with the timing of marking the trunk requests as complete - that did change a number of times to get all the relevant structure lifetimes correct.
Otherwise in RDEBUG logging it is not clear which trunk the log relates to.
The request being requeued is part of the list of requests causing the connection to be marked as full - so it should be allowed to requeue.
The same structures are used for admin binds and user binds, but some elements are only used in admin binds.
And associated function to allocate / retrieve the trunk
Since only one bind can be in progress at a time on a given connect, use of a trunk allows the number of connections in use to scale with load.