progress tracking for keep alive reply #64
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this way we can find what wal_end are still being processed This will be useful if some transaction have long processing time and to improve the keep-alive reply
…ssed Currently we always return the server's wal_end+1 This approach has a few limits, can cause some issues. If we start processing wal_end A, then receive a keep-alive request with wal_end=A+x and reply A+x+1, this before A if finished processing, then if our process crashes the server will restart at A+x+1 and will never receive it again. To avoid this, this PR returns wal_end+1 only if there are no processing transactions if there are it returns the wal_end of the oldest transaction still in progress. This way we will be guaranteed to start back from there(A and not A+x+1) if we crash.
|
oh forgot that the tests uses |
|
This is a good idea, what else is needed? I just merged your other small tweak branch. |
|
oh sorry, I thought I had sent a comment with the closure of the PR There a few things where I'm not enough certain of the outcome. It's not that clear but I'm afraid that replying with the current wal_end +1 for the first field will cause Postgres re-send packets. |
|
I see. Do you mind opening a Discussion on this topic? Maybe others have some insights. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
context:
When receiving a keep-alive request from Postgres, replying with Posgres' current
wal_end + 1notifies the Postgres server that we fully processed up-to its
wal_end+1.issue:
As Replication.Server communicates with Replication.Publisher asynchronously this means that,
if an error occurs during the processing of a message,
we can acknowledged many more messages than we actually processed.
For a durable slot, this means that when the durable slot will restart, it will do so at the last
wal_end+1we replied with (loosing events).
The longer the transaction takes to be fully processed (many records, etc.) the higher the risk of this happening
solution proposed:
Adding a Replication.Progress Agent that stores in a :gb_sets (ordered set) the LSN of transactions.
When we start receiving the transaction it we push it
We drops it when the processing is done.
In the Replication.Server, we then only need to get the wal_end of the smallest LSN in progress as keep-alive reply.
If no transaction is in progress we can return the received wal_end+1 instead as currently.