Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[walproposer] Async WAL append #105

Merged
merged 9 commits into from
Dec 21, 2021
Merged

[walproposer] Async WAL append #105

merged 9 commits into from
Dec 21, 2021

Conversation

petuhovskiy
Copy link
Member

@petuhovskiy petuhovskiy commented Dec 1, 2021 •
edited
Loading

Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.

@petuhovskiy petuhovskiy mentioned this pull request Dec 7, 2021
Closed
Base automatically changed from term_history to main December 7, 2021 15:47
@petuhovskiy petuhovskiy marked this pull request as ready for review December 14, 2021 14:26
@petuhovskiy petuhovskiy requested a review from arssher December 14, 2021 14:26
@petuhovskiy petuhovskiy mentioned this pull request Dec 14, 2021
Merged
@petuhovskiy petuhovskiy force-pushed the append_ahead branch 3 times, most recently from 707ce19 to 4aab715 Compare December 15, 2021 11:57
arssher
arssher requested changes Dec 21, 2021
View reviewed changes
src/backend/replication/walproposer.c Outdated
@@ -720,27 +727,14 @@ SendMessageToNode(int i, WalMessage *msg)
msg = msg->next;

wk->currMsg = msg;
wk->flushWrite = false;
wk->state = SS_ACTIVE;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to StartStreaming, as we need it only once

src/backend/replication/walproposer.c Outdated
*/
wk->state = SS_SEND_WAL;
/* Don't ned to update the event set; that's done by AdvancePollState */
/* TODO: do we need to send messages immediately? */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ok, we can convert comment to smth like 'Note: we always send everything to the safekeeper until WOULDBLOCK or nothing left to send'

src/backend/replication/walproposer.c Outdated
wk->state = SS_IDLE;
UpdateEventSet(wk, WL_SOCKET_READABLE);
}
UpdateEventSet(wk, WL_SOCKET_READABLE | (wk->flushWrite ? WL_SOCKET_WRITEABLE : 0));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant? SendAppendRequests guts will set WL_SOCKET_WRITEABLE if needed.

src/backend/replication/walproposer.c Outdated
* SendAppendRequests, but sent through a wire only with a flush in
* reading routine.
*/
while (wk->ackMsg != NULL && wk->ackMsg != wk->currMsg)
Copy link

@arssher arssher Dec 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this coding, if we don't wait for anything and connection is lost, we won't try reading from it and thus will never reset it, right? So better prepare to async responses right away and just expect here any ack message.

Also this 'sent with a flush in reading routine' shouldn't happen anymore...

On the second thought, it is not that bad as in case of any required action (data to send) connection reset will happen; better fix it right now, but I'm ok with doing it separately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'sent with a flush in reading routine' still can happen, it's in PQConsumeInput implementation: https://github.com/zenithdb/postgres/blob/main/src/interfaces/libpq/fe-exec.c#L1909-L1919

As we handle all read/write events and don't need to flush in PQConsumeInput, we may try to switch to using pqReadData instead. But for now we still can read responses ahead of requests, so I'd better create an issue for better async handling and fix it later.

@petuhovskiy petuhovskiy merged commit 22938e1 into main Dec 21, 2021
@petuhovskiy petuhovskiy deleted the append_ahead branch December 21, 2021 13:51
lubennikovaav pushed a commit that referenced this pull request Feb 9, 2022
@petuhovskiy @lubennikovaav
[walproposer] Async WAL append (#105)
9bbc511 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
MMeent pushed a commit that referenced this pull request Jul 7, 2022
@petuhovskiy @MMeent
[walproposer] Async WAL append (#105)
774187e 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
MMeent pushed a commit that referenced this pull request Aug 18, 2022
@petuhovskiy @MMeent
[walproposer] Async WAL append (#105)
cb56ff6 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
lubennikovaav pushed a commit that referenced this pull request Nov 21, 2022
@petuhovskiy @lubennikovaav
[walproposer] Async WAL append (#105)
145a439 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
MMeent pushed a commit that referenced this pull request Feb 10, 2023
@petuhovskiy @MMeent
[walproposer] Async WAL append (#105)
046d824 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
MMeent pushed a commit that referenced this pull request Feb 10, 2023
@petuhovskiy @MMeent
[walproposer] Async WAL append (#105)
4d089bb 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
MMeent pushed a commit that referenced this pull request May 11, 2023
@petuhovskiy @MMeent
[walproposer] Async WAL append (#105)
0adee17 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
tristan957 pushed a commit that referenced this pull request Aug 10, 2023
@petuhovskiy @tristan957
[walproposer] Async WAL append (#105)
425c935 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
@petuhovskiy @tristan957
[walproposer] Async WAL append (#105)
e85d91f 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
@petuhovskiy @tristan957
[walproposer] Async WAL append (#105)
a8580f0 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
@petuhovskiy @tristan957
[walproposer] Async WAL append (#105)
533678a 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
@petuhovskiy @tristan957
[walproposer] Async WAL append (#105)
6dd00f1 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
tristan957 pushed a commit that referenced this pull request Feb 6, 2024
@petuhovskiy @tristan957
[walproposer] Async WAL append (#105)
7c5e028 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
tristan957 pushed a commit that referenced this pull request May 10, 2024
@petuhovskiy @tristan957
[walproposer] Async WAL append (#105)
69ee4ef 
Implement async wp <-> sk protocol, send WAL messages ahead of feedback replies.

New SS_ACTIVE state is introduced instead of former SS_SEND_WAL / SS_SEND_WAL_FLUSH / SS_RECV_FEEDBACK.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arssher arssher arssher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@petuhovskiy @arssher