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

Why do we need to lock? #558

Open
fade2black opened this issue Dec 23, 2024 · 5 comments
Open

Why do we need to lock? #558

fade2black opened this issue Dec 23, 2024 · 5 comments
Labels
Question A question to be answered.

Comments

@fade2black
Copy link

fade2black commented Dec 23, 2024

Hi! I don't understand why we need to wrap the core storage inside Arc<RwLock<>> (in MemStorage example). I do understand what Arc and RwLock do, locking and exclusive write access, etc, (so no need explanation), but why do we need an exclusive access? Doesn't each node have its own log storage? Or do nodes may access their log storage asynchronously from a several threads simultaneously?

#[derive(Clone, Default)]
pub struct MemStorage {
    core: Arc<RwLock<MemStorageCore>>,
}
@BusyJay
Copy link
Member

BusyJay commented Dec 23, 2024

Lock is not required. MemStorage is just an example implementation for tests and explanation.

@fade2black
Copy link
Author

fade2black commented Dec 23, 2024

If Lock is not required then why do you use Arc and RwLock? Looks strange and and confusing.
I am curious about the reason you use these two synchronization.

tests and explanation

What does it explain?

I have checked another implementation of the Raft (openraft), they also use Arc and Mutex. When I asked them why they use, they said the same thing: "Locking is not necessary, just for example".
Then, if it is not necessary, why do you use locking? It is not just a dummy field inside a struct or unused variable. When you use locking mechanism and acquire lock guards, people naturally think that you synchronise the access and and try to find the reason behind it in your code. I have spent entire night trying to find the reason, and you simply "Lock is not required. MemStorage is just an example implementation for tests and explanation.". :-) Really weird.

@fade2black
Copy link
Author

As far as I understand each node has its own log storage. Suppose a leader has been elected, and nodes starts to communicate. I also know that nodes constantly read from their log storage and update their log storage (execute command, etc). So, don't we need to synchronise log storage updates?

@BusyJay
Copy link
Member

BusyJay commented Dec 23, 2024

In Rust, you should utilize Sync and Send traits for thread safety consideration instead of locks. Both former traits implies how is a trait/struct used, lock is just an implementation details. Storage trait doesn't require Sync or Send, means it's completely OK to be implemented without Lock. However, raft-rs doesn't require a sync version storage, application that uses raft-rs may want one.

don't we need to synchronise log storage updates?

That's not what raft-rs concerns. Raft-rs focus on the algorithm part, other side effects like sending messages, persisting logs and replaying logs are exposed to application via ready function. It's up to application to decide whether write logs concurrently or not, sending messages using TCP or UDP, etc.

@BusyJay BusyJay added the Question A question to be answered. label Dec 23, 2024
@fade2black
Copy link
Author

fade2black commented Dec 23, 2024

Thanks, got you. I think it'd be very helpful if you could put more explanation regarding what part of raft you implement and what part is up to client. :-) As far as I understand the algorithm, storing in logs and reading from logs ARE also a part of the raft algorithm. I don’t think it is optional and application specific. I have been exploring both rust implementations (especially docs and examples), at this point I have decided that implementing myself from scratch would be easier than trying to understand your implementations and “examples”. Sad but true. In fact this is the case with entire Rust ecosystem. 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question A question to be answered.
Projects
None yet
Development

No branches or pull requests

2 participants