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

rocksdb ingest causes the leader lease to be invalid #5265

Closed
tangyuanzhang opened this issue Jan 16, 2023 · 5 comments
Closed

rocksdb ingest causes the leader lease to be invalid #5265

tangyuanzhang opened this issue Jan 16, 2023 · 5 comments
Assignees
Labels
affects/none PR/issue: this bug affects none version. process/fixed Process of bug severity/none Severity of bug type/bug Type: something is unexpected

Comments

@tangyuanzhang
Copy link
Contributor

tangyuanzhang commented Jan 16, 2023

The nebula heartbeat is divided into two steps:
Step 1: If replicatingLogs_ is false, then the leader synchronizes an empty log of the follower, and then writes it to rocksdb
Step 2: Initiate rpc, synchronize current information, do not write rocksdb,

And both steps will update lastMsgAcceptedCostMs_ and lastMsgAcceptedTime_
, the condition for the leader to judge that the lease is valid is:
time::WallClock::fastNowInMilliSec() - lastMsgAcceptedTime_ <
FLAGS_raft_heartbeat_interval_secs * 1000 - lastMsgAcceptedCostMs_;

When rocksdb is ingesting, trigger write stall, which will block writing. At this time, the empty log in step 1 cannot be written to rocksdb, and replicatingLogs_ cannot be updated false.
Each execution of sendHeartbeat() will only execute step 2. When the write stall disappears, the blocked write will be completed, and the lastMsgAcceptedTime_ and lastMsgAcceptedCostMs_ (very large) will be updated at the same time. At the same time, an error will be reported when querying this part leader: E_LEADER_LEASE_FAILED。

I think it needs to be judged when it should be updated. Only the latest AppendLog can update the data.
image

@tangyuanzhang tangyuanzhang added the type/bug Type: something is unexpected label Jan 16, 2023
@github-actions github-actions bot added affects/none PR/issue: this bug affects none version. severity/none Severity of bug labels Jan 16, 2023
@critical27
Copy link
Contributor

Good catch~
I suppose we check compare nowTime and lastMsgAcceptedTime_ first:

  • if nowTime is bigger, compare nowTime - nowCosMs and lastMsgAcceptedTime_ - lastMsgAcceptedCostMs_ as the code you posted above
  • if lastMsgAcceptedTime_ is bigger, just do nothing

Would you like to contribute the a pull request?

BTW, what is the size of sst that you ingest?

@tangyuanzhang
Copy link
Contributor Author

I think there is no need to compare nowTime and lastMsg Accepted Time, the leader updates lastMsgAcceptedTime_ is to lock raftLock_, so here nowTime > lastMsgAcceptedTime_ is always true。
image

@tangyuanzhang
Copy link
Contributor Author

I will contribute the a pull request 。
The ingest sst file is 200G, and the storage data is 1T。

@wey-gu
Copy link
Contributor

wey-gu commented Jan 17, 2023

Dear @ShiXiangZ ,

We would like to send you a gift for the "good catch" NebulaGraph community award(and for the contributor award when PR is merged), would you mind sending us a mail to [email protected] with your address so that can receive the gift shipment?

Thanks and welcome to the community!

cc @QingZ11 @lisahui

@qiwei9743
Copy link

What we should do is to update a max point-in-time lease time = lastMsgAcceptedTime_ - lastMsgAcceptedCostMs_ + FLAGS_raft_heartbeat_interval_secs. FLAGS_raft_heartbeat_interval_secs is a constant. The comparison is only related to lastMsgAcceptedTime_ - lastMsgAcceptedCostMs_.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/none PR/issue: this bug affects none version. process/fixed Process of bug severity/none Severity of bug type/bug Type: something is unexpected
Projects
None yet
Development

No branches or pull requests

5 participants