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

fix: race condition when reading and writing to inflight queue #116

Merged
merged 12 commits into from
Apr 24, 2024

Conversation

EnriqueL8
Copy link
Contributor

The policy loop frequently read and writes from the inflight queue, more specifically:

  • It will load new transactions into the queue when it has space in updateInFlightSet and remove the ones that have completed.
  • Remove transactions from the queue when it gets enough confirmations for that transaction

When loading new transactions, the code will take a copy and override the inflight queue with an empty array. If another thread was reading from such array, most often iterating to find a specific inflight tx it wouldn't find it.

This PR alongside with adding extra logging to help diagnose this sort of issues, adds a read/write mutex for the inflight queue and a mutex for each pendingState to be thread safe. Creates read locks when iterating and reading from the queue, write lock in a single place where we remove or load new transactions in queue and a mutex on the pendingState when we read or modify it.

Draft: as running another performance test for the latest commit

Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
@EnriqueL8 EnriqueL8 marked this pull request as ready for review April 24, 2024 10:22
@EnriqueL8 EnriqueL8 requested a review from a team as a code owner April 24, 2024 10:22
Copy link
Contributor

@Chengxuan Chengxuan left a comment

Choose a reason for hiding this comment

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

Minor comments on the loggings. The use of read locks looks good.

pkg/txhandler/simple/policyloop.go Outdated Show resolved Hide resolved
pkg/txhandler/simple/policyloop.go Outdated Show resolved Hide resolved
pkg/txhandler/simple/simple_transaction_handler.go Outdated Show resolved Hide resolved
pkg/txhandler/simple/policyloop.go Outdated Show resolved Hide resolved
EnriqueL8 and others added 3 commits April 24, 2024 12:36
Co-authored-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
Signed-off-by: Enrique Lacal <enriquelacal@gmail.com>
Co-authored-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
Signed-off-by: Enrique Lacal <enriquelacal@gmail.com>
Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>
Copy link
Contributor

@Chengxuan Chengxuan left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
@Chengxuan Chengxuan merged commit af4a223 into hyperledger:main Apr 24, 2024
2 checks passed
@Chengxuan Chengxuan deleted the add_logs branch April 24, 2024 12:11
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