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

Update TestProvideSnap2C in raft_test.go #425

Open
wants to merge 1 commit into
base: course
Choose a base branch
from
Open

Update TestProvideSnap2C in raft_test.go #425

wants to merge 1 commit into from

Conversation

niebayes
Copy link
Contributor

@niebayes niebayes commented Sep 6, 2022

handleSnapshot only affects states in raft, not affects states in storage. More specifically, the snapshot in memory storage will be as it is after handleSnapshot. As a result, when leader is willing to send a snapshot to follower, it is the empty snapshot being sent.
However, in my implementation, such a sending will be aborted since the snapshot is empty. Therefore, this test fails.
Sending an empty snapshot is definitely shall be rejected, and I update the test case to not confuse ones like me.

`handleSnapshot` only affects states in raft, not affects states in storage. More specifically, the snapshot in memory storage will be as it is after `handleSnapshot`. As a result, when leader is willing to send a snapshot to follower, it is the empty snapshot being sent. 
However, in my implementation, such a sending will be aborted since the snapshot is empty. Therefore, this test fails.
Sending an empty snapshot is definitely shall be rejected, and I update the test case to not confuse ones like me.
Copy link
Collaborator

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

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

why can't it send an empty snapshot?

@niebayes
Copy link
Contributor Author

niebayes commented Sep 6, 2022

In etcd's implementation, a panic is raised if the storage gives back an empty snapshot. A snapshot is regarded empty if its metadata index is 0.
In the TestProvideSnap2C test, the storage interface is implemented by the memory storage. On receiving a Snapshot request, it simply gives back the snapshot it created on init, which is a snapshot with metadata index equals to 0. That's an empty snapshot.
Such a snapshot is rejected in etcd, and tinykv is ported from etcd. Why would tinykv send it otherwise?

@niebayes niebayes closed this by deleting the head repository Oct 9, 2022
@Connor1996 Connor1996 reopened this Oct 10, 2022
@Connor1996
Copy link
Collaborator

Connor1996 commented Oct 10, 2022

I misunderstood the meaning of empty snap. As expected, you shouldn't return the init snap for MemStorage

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