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

3966 Remove clone from Membership and wrap it in Arc<RwLock>> #3976

Merged
merged 1 commit into from
Dec 19, 2024
Merged

Conversation

pls148
Copy link
Contributor

@pls148 pls148 commented Dec 17, 2024

Closes #3966

This PR:

Converts internal storage of Membership object from a copied-around entity to a singular object wrapped in RwLock.

This PR does not:

Key places to review:

Testing code. It got a little tricky to make sure that each Membership object for each node being tested was wrapped in its own RwLock rather than having all of the nodes share the same Membership object

Copy link
Contributor

@lukaszrzasik lukaszrzasik left a comment

Choose a reason for hiding this comment

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

I think the change is tricky as we now have a shared instance accessed in many different places. I have some comments. These might be important. Let me know what you think.

crates/examples/infra/mod.rs Outdated Show resolved Hide resolved
crates/task-impls/src/network.rs Outdated Show resolved Hide resolved
crates/task-impls/src/response.rs Outdated Show resolved Hide resolved
crates/task-impls/src/transactions.rs Outdated Show resolved Hide resolved
crates/task-impls/src/transactions.rs Outdated Show resolved Hide resolved
crates/task-impls/src/upgrade.rs Outdated Show resolved Hide resolved
crates/task-impls/src/vote_collection.rs Outdated Show resolved Hide resolved
crates/testing/src/helpers.rs Show resolved Hide resolved
crates/testing/src/helpers.rs Show resolved Hide resolved
Copy link
Contributor

@lukaszrzasik lukaszrzasik left a comment

Choose a reason for hiding this comment

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

I think it's good to go.

@pls148 pls148 merged commit 71cf05d into main Dec 19, 2024
17 checks passed
@pls148 pls148 deleted the ps/3966 branch December 19, 2024 16:04
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.

Membership trait changes to support PoS
4 participants