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

writes epoch-slots to crds table synchronously #17719

Merged
merged 2 commits into from
Jun 4, 2021

Conversation

behzadnouri
Copy link
Contributor

Problem

epoch-slots may be overwritten before they are written to crds table:
#17711

Summary of Changes

This commit writes new epoch-slots to crds table synchronously with
push_epoch_slots. The functions is still not thread-safe as commented in
the code, however currently only one threads is invoking this code.

epoch-slots may be overwritten before they are written to crds table:
solana-labs#17711

This commit writes new epoch-slots to crds table synchronously with
push_epoch_slots. The functions is still not thread-safe as commented in
the code, however currently only one threads is invoking this code.
@carllin
Copy link
Contributor

carllin commented Jun 3, 2021

Awesome, thanks, this unblocks me 😃

Can we also remove the flushes in this test here: https://github.com/solana-labs/solana/blob/master/gossip/src/cluster_info.rs#L4137-L4139 (the test passes now without those flushes) to enforce this guarantee that sequential writes don't overwrite each other even without an intermediary flush?

carllin
carllin previously approved these changes Jun 3, 2021
@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #17719 (df33e6b) into master (708bbcb) will decrease coverage by 0.0%.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #17719     +/-   ##
=========================================
- Coverage    82.7%    82.7%   -0.1%     
=========================================
  Files         431      431             
  Lines      120590   120590             
=========================================
- Hits        99839    99835      -4     
- Misses      20751    20755      +4     

@behzadnouri
Copy link
Contributor Author

Can we also remove the flushes in this test here: https://github.com/solana-labs/solana/blob/master/gossip/src/cluster_info.rs#L4137-L4139 (the test passes now without those flushes) to enforce this guarantee that sequential writes don't overwrite each other even without an intermediary flush?

good point, done

@mergify mergify bot dismissed carllin’s stale review June 4, 2021 12:01

Pull request has been modified.

@behzadnouri behzadnouri merged commit 60b0a13 into solana-labs:master Jun 4, 2021
@behzadnouri behzadnouri deleted the push-epoch-slots-race branch June 4, 2021 13:56
mergify bot pushed a commit that referenced this pull request Jun 4, 2021
epoch-slots may be overwritten before they are written to crds table:
#17711

This commit writes new epoch-slots to crds table synchronously with
push_epoch_slots. The functions is still not thread-safe as commented in
the code, however currently only one threads is invoking this code.

(cherry picked from commit 60b0a13)
mergify bot added a commit that referenced this pull request Jun 4, 2021
epoch-slots may be overwritten before they are written to crds table:
#17711

This commit writes new epoch-slots to crds table synchronously with
push_epoch_slots. The functions is still not thread-safe as commented in
the code, however currently only one threads is invoking this code.

(cherry picked from commit 60b0a13)

Co-authored-by: behzad nouri <[email protected]>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
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