Improve broker initiated client disconnects #816
Conversation
spuun
force-pushed
the
bugfix-force-close-socket-on-broker-initiated-connection-close
branch
from
cffea6d
to
f8a0fca
Compare
|
One thing we don't get now is the "async" close of ALL connections on shutdown. If there are multiple "zombies" on shutdown we'll have to wait |
|
Yes, but im thinking we can have that logic elsewhere, like push all connections to a Channel and then let a (limited) amount of fibers to work it off until it's empty. |
spuun
force-pushed
the
bugfix-force-close-socket-on-broker-initiated-connection-close
branch
from
57efbec
to
7961ff6
Compare
spuun
changed the title
Client#close to force close socket
|
Updated the title and description. I've added the solution with fibers and a channel to disconnects clients "async" on shutdown. |
src/lavinmq/vhost.cr
Outdated
| else | ||
| fiber_id = fiber_count &+= 1 | ||
| @log.trace { "spawning close conn fiber #{fiber_id} " } | ||
| wg.spawn do |
There was a problem hiding this comment.
We could make a PR to the stdlib so that we can name fibers spawned by WaitGroup#spawn.
src/lavinmq/vhost.cr
Outdated
| end | ||
| @log.trace { "exiting close conn fiber #{fiber_id} " } | ||
| end | ||
| Fiber.yield |
There was a problem hiding this comment.
Required? Doesn't Channel#send on a unbuffered channel automatically yield the current fiber?
There was a problem hiding this comment.
I'm not sure why, but it did spawn one extra fiber. Not a big deal, but...
spuun
deleted the
bugfix-force-close-socket-on-broker-initiated-connection-close
branch
WHAT is this pull request doing?
Writing to client sockets may block a fiber because the buffers are full. When this happens this fiber will hold the write lock in
Clientuntil it manage to write the data. Because if this it's not possible to force close a connection from the management UI, and the http request will timeout. It will also affect lavinmq shutdown becauseVHost#closewill be blocked from closing all connections effectively.This PR will:
#closethat will set awrite_timeouton the socket.HOW can this pull request be tested?
Manually with a "bad" client, e.g. unlimited prefetch and no acking. Let it consume until LavinMQ stops because of tcp back pressue. Try kill the connection.