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.

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

Simplify walproposer code #114

Merged
merged 6 commits into from
Jan 4, 2022
Merged

Simplify walproposer code #114

merged 6 commits into from
Jan 4, 2022

Conversation

petuhovskiy
Copy link
Member

@petuhovskiy petuhovskiy commented Dec 30, 2021

Closes neondatabase/neon#1041

  • Leave only states in which we wait, others can be executed directly or in a function
  • Remove loop in AdvancePollState
  • Migrate AsyncReadFixed to AsyncReadMessage
  • Once connection is established, do UpdateEventSet only when we need to flush (set WL_SOCKET_WRITEABLE) or when flush is done (unset it).
  • Optimize UpdateEventSet calls

@arssher
Copy link

arssher commented Dec 30, 2021

Approve on first two checkboxes.

Base automatically changed from reorder-walproposer-code to main December 31, 2021 09:57
@petuhovskiy petuhovskiy marked this pull request as ready for review December 31, 2021 13:31
Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.
@petuhovskiy
Copy link
Member Author

Updating event set only on flushes is tricky, I optimized UpdateEventSet calls in SS_ACTIVE differently. Now all SS_ACTIVE code is executed only with calls to HandleActiveState, which calls UpdateEventSet in the end. This way we have single place for updating event set and it will not flip in the middle of the handler. Also, if event mask is not updated in the next call to UpdateEventSet, no extra calls will be done, because ModifyWaitEvent has early exit check for such case.

Flushes are tricky mostly because we can have a call stack like HandleActiveState -> RecvAppendResponses -> BroadcastMessage -> SendMessageToNode -> HandleActiveState -> SendAppendRequests. In this case we should be careful about overriding event set, which can cause unsetting WL_SOCKET_WRITEABLE while we still have messages to send.

@petuhovskiy petuhovskiy requested a review from arssher January 3, 2022 13:03
Copy link

@arssher arssher left a comment

Choose a reason for hiding this comment

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

After looking for some time I now think that UpdateEventSet placement here is better than what I initially proposed :)

@petuhovskiy
Copy link
Member Author

Another thing I remembered, that after HackyRemoveWalProposerEvent (happens on every reconnect) we always UpdateEventSet(WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE) for SS_ACTIVE, because that's how it's defined in WalKeeperStateDesiredEvents.

HandleActiveState isn't picky on events and will fix them in any case, but that's still extra work on every reconnect.

@arssher
Copy link

arssher commented Jan 4, 2022

Not that it is extremely expensive, but yeah, it would be nicer to check for flushWrite in WalKeeperStateDesiredEvents to fix it.

@petuhovskiy petuhovskiy merged commit 662257d into main Jan 4, 2022
@petuhovskiy petuhovskiy deleted the simplify-walproposer branch January 4, 2022 10:29
lubennikovaav pushed a commit that referenced this pull request Feb 9, 2022
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
MMeent pushed a commit that referenced this pull request Jul 7, 2022
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
MMeent pushed a commit that referenced this pull request Aug 18, 2022
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
lubennikovaav pushed a commit that referenced this pull request Nov 21, 2022
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
MMeent pushed a commit that referenced this pull request Feb 10, 2023
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
MMeent pushed a commit that referenced this pull request Feb 10, 2023
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
MMeent pushed a commit that referenced this pull request May 11, 2023
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
tristan957 pushed a commit that referenced this pull request Aug 10, 2023
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
tristan957 pushed a commit that referenced this pull request Feb 6, 2024
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
tristan957 pushed a commit that referenced this pull request May 10, 2024
* Clean up walproposer states

* Migrate AsyncReadFixed to AsyncReadMessage

* Handle flushWrite better a bit

* Update SS_ACTIVE event set in single place

Now event set is updated only in the end of HandleActiveState, after
all handlers code was executed.

* Add comment on SS_ACTIVE write event

* Add TODO for SS_ACTIVE DesiredEvents
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify walproposer code
2 participants