-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: persist inflight publishes and send on restart #299
Conversation
@Vilayat-Ali take a look at the changes(commit wise) and write the document of approach you took, the changes I made to the same and what you learnt from the process. |
4b89737
to
5c85fd4
Compare
* Implemeted reading from disk and publishing onto the network * inflight persistence logic * doc: make the code readable --------- Co-authored-by: Devdutt Shenoi <devdutt@bytebeam.io>
Let's consider this situation with regards to full-graceful shutdown:
This is something that has been bugging me for a while looking at the code. Illustrating the scenario, consider 10 unacked publishes are (A, B) and 10 in_channel publishes are (C, D), first shutdown of uplink writes them to inflight file as (A, B, C, D), on restart only first 10(i.e. A, B) publishes are loaded while rest 10(C, D) are not. The two possible ways to handle next shutdown would be:
NOTE: those 10 publishes in_channel after restart will now avalanche if we had uplink restarts continuously(A,B,C,D,A,B,A,B...), while only 10 packets will remain lost in the other scenario(A,B). The second path is what I have taken in the current implementation, but wanted to document this decision and ask for feedback. |
Code to ease readability of control handles as requested by @tekjar is included in the PR #316 and this commit |
7bd9d29
to
9a89646
Compare
9a89646
to
e27cb0e
Compare
Uplink always accepts packets read out of the inflight file, we don't have to fret about the situation illustrated in the previous comment. proof:
This is despite network never being back up. |
I have opted to just load pending publishes back into By making the channel between Some of the other commits above also significantly improve upon the readability of relevant parts of the code and ensure we don't end up copying too much. We are now dependent on bytebeamio/rumqtt#780 to ensure correct working of |
…/graceful-shutdown
Closes #
Changes
Why?
Allows the device to go down and come back up without losing un-acked publishes or storage that hasn't overflowed yet. This is done by pushing onto disk whatever is waiting inflight not overflowed in-memory.
Trials Performed
Run against
qa-scripts/persistence.sh
on bad downstream, allowing us to force packets to be not acked and hence remain inflight, waiting to be written to disk.The
/var/tmp/persistence/inflight
file contains:And these publishes inflight/persisted are resent to the
EventLoop
on uplink restart: