[I] Support rate-limited session disconnection for graceful node draining [bifromq]

["syh194194 (via GitHub)" <gi...@apache.org>]

syh194194 opened a new issue, #254:
URL: https://github.com/apache/bifromq/issues/254

     Is your feature request related to a problem? Please describe.
     
     When performing rolling updates on large BifroMQ clusters, operators need to drain connections from a node before taking it offline. The only available mechanism is DELETE /kill, which disconnects all matching sessions simultaneously with no rate control. On a node with hundreds of thousands of
     connections, this causes a sudden reconnection storm: all devices receive TCP FIN at the same moment and immediately retry, creating a spike that can overload the remaining nodes and trigger cascading failures.
   
     The codebase already acknowledges this gap with an explicit TODO comment at `bifromq-session-dict/bifromq-session-dict-server/src/main/java/.../SessionDictService.java:102:`
   
   ```
     // TODO: support disconnect a constant rate
     sessionRegistrations.forEach(reg -> reg.stop(request.getKiller(), request.getServerRedirection()));
   ```
   
     Describe the solution you'd like
   
     Implement the TODO: add an optional rate field to KillAllRequest that limits how many sessions are disconnected per second. A value of 0 preserves the current behavior (no rate limit) for backwards compatibility.
   
     Proposed changes across 4 files:
     
     SessionDictService.proto — add field to KillAllRequest:
   `  uint32 rate = 6; // disconnect rate per second, 0 = unlimited (default)`
     
     SessionDictService.java — replace the forEach with a rate-limited loop:
   ```
     // replace the TODO line with:
     RateLimiter limiter = (request.getRate() > 0)
         ? RateLimiter.create(request.getRate()) : null;
     for (ISessionRegistry.SessionRegistration reg : sessionRegistrations) {
         if (limiter != null) limiter.acquire();
         reg.stop(request.getKiller(), request.getServerRedirection());
     }   
   ```
     
     ISessionDictClient.java — add rate parameter with a default method to keep backwards compatibility.
   
     KillHandler.java — read an optional rate header and pass it through to the client call.
   
     The resulting API call would look like:
   ```
   
     # Drain a node at 500 disconnections/second
     curl -X DELETE http://<node-ip>:8091/kill \
       -H "tenant_id: <tenant>" \
       -H "rate: 500"
   ```
   
     Since a rate-limited kill may run for a long time (e.g. 1,000,000 connections / 500 per second = 2000 seconds), the implementation should either increase the gRPC timeout accordingly or execute asynchronously and return 202 Accepted immediately.
   
     Describe alternatives you've considered
   
     - External script calling DELETE /kill per client-id one by one: Requires a separate API to list all sessions on a node, which does not currently exist. Also introduces network round-trip overhead for every individual disconnect.
     - NLB weight-based draining without forced disconnect: New connections stop arriving, but existing connections stay until the device disconnects naturally. This is too slow for planned maintenance windows.
     - Abrupt killAll (current behavior): Works but causes a reconnection storm. Acceptable only for small deployments or off-peak windows.
   
     Additional context
   
     The RateLimiter from Guava is already a transitive dependency in the project, so no new dependency is needed. The change is entirely backward-compatible: omitting the rate header gives identical behavior to today.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@bifromq.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Support rate-limited session disconnection for graceful node draining [bifromq]

["popduke (via GitHub)" <gi...@apache.org>]

popduke commented on issue #254:
URL: https://github.com/apache/bifromq/issues/254#issuecomment-4446720197

   One thing I’m curious about: why would DELETE /kill be used in a rolling upgrade flow?
   
   My understanding is that this API is for kicking sessions by tenant, user, or client, and its semantics don’t include draining all connections from a specific broker node.
   
   For rolling upgrades, wouldn’t it be more appropriate to configure a suitable maxDisconnPerSec and let the broker control the disconnect rate during graceful shutdown? That path seems closer to node draining than extending the kill API.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@bifromq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org