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

node/consensus: priority lock for CE business to preempt tx influx #1430

Merged
merged 2 commits into from
Mar 4, 2025

Conversation

jchappelow
Copy link
Member

This adds node/consensus.PriorityLock that is like a normal mutex but:

  • Lock guarantees FIFO when acquired, no issues with sleeping goroutines in the sync.Mutex breaking ordering
  • PriorityLock jumps the line, getting the lock ahead of others that used Lock

This is intended in the very specific use case of CE where the mempoolMtx was locked in:

  • commit: block commit, priority
  • prepareBlockProposal: leader preparing block proposal, priority
  • QueueTx: ingest new tx from either p2p announce or RPC broadcast, NOT priority

All of the "priority" cases are sequential, never concurrent with each other. Only the QueueTx spot can be concurrent and at any time, as it is triggered from external sources.

This eliminate the one bad spot of mutex contention revealed by running (for a brief period) with --profile-mode=mutex while stress tool is flooding the network.

@jchappelow jchappelow marked this pull request as ready for review March 3, 2025 18:16
charithabandi
charithabandi previously approved these changes Mar 3, 2025

import "sync"

type PriorityLockQueue struct {
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to doc and clean up this PR.

@jchappelow jchappelow changed the title node/consensus: priority lock for CE business to preemtpy tx influx node/consensus: priority lock for CE business to preempt tx influx Mar 3, 2025
@charithabandi charithabandi merged commit f206dbd into kwilteam:main Mar 4, 2025
2 checks passed
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.

2 participants