[FIXED] Fail of publish ack on connection close #298
Conversation
|
@derekcollison This is all tests fixes from #297 and simply the fix for the pub acks on connection close. We could use this instead of #297 if we think that changes on that other PR are a bit too much (I can keep the branch to revisit later). As I commented on the other PR, the number of outstanding timers should be limited by the number of |
We can't invoke the ack handler from the connection close code since there would be a possible deadlock if user code is calling any connection method. This is based out of #297 but without the change on how pub acks are timed-out (still use timers). So this PR has only tests fixes and the fix for failing pub acks on connection close. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
kozlovic
force-pushed
the
fix_pub_acks_on_conn_close
branch
from
8f93045
to
f23d453
Compare
|
@derekcollison Apologies, I have updated (push force) the PR. Only stan.go was changed. I realized that doing the timer.Reset() would work, but then the error would be ErrTimeout, which in this case ought to be ErrConnectionClosed. So we now collect all the pub acks that have a timer set and can be stopped and then invoke their ack handlers in separate go routine. Also, once you review this version, we should decide which PR we use, this one or #297. We can close the associated issue and say that we don't think the number of timers will be an issue (due to MaxPubAcksInflight and with possible improvement in Go 1.14... |
|
Agreed! |
We can't invoke the ack handler from the connection close code
since there would be a possible deadlock if user code is calling
any connection method.
This is based out of #297 but without the change on how pub acks
are timed-out (still use timers). So this PR has only tests fixes
and the fix for failing pub acks on connection close.
Signed-off-by: Ivan Kozlovic ivan@synadia.com