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

raftstore, apply: catch up logs for merge through raft #4595

Merged
merged 31 commits into from Jul 30, 2019

Conversation

@Connor1996
Copy link
Member

Connor1996 commented Apr 29, 2019

What have you changed? (mandatory)

Fix #4581 and #4560

Before the target region sends log entries to source region's apply delegate to apply. Notice that, these logs are not appended to raft machine, so if TiKV restarts after applying the logs and before the source region is set to TombStone, raft machine may find that apply_index > commit_index and panic.

Now, we send these logs back to raftstore and wrap them as RaftMessage::MsgAppend, and then applying these logs in the normal process way. Also, it keeps apply worker from handling some cascade merge scenery, so I remove CatchUpLogs inWaitMergeState.

The process order would be:

  1. exec_commit_merge in target apply worker
  2. catch_up_logs_for_merge in source apply worker (chech whether need to catch up logs)
  3. on_ready_catch_up_logs in source raftstore
  4. ... (raft append and apply logs)
  5. on_ready_prepare_merge in source raftstore (means source region has finished appling all logs)
  6. catch_up_logs_for_merge in source apply worker(send LogsUpToDate)
  7. exec_commit_merge in target apply worker
  8. on_ready_commit_merge in target raftstore

What are the type of the changes? (mandatory)

  • Bug fix (change which fixes an issue)

How has this PR been tested? (mandatory)

unit-test

Does this PR affect documentation (docs) or release note? (mandatory)

No

Does this PR affect tidb-ansible update? (mandatory)

No

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Connor1996 added 2 commits Apr 29, 2019
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Apr 29, 2019

/release

src/raftstore/store/fsm/peer.rs Outdated Show resolved Hide resolved
src/raftstore/store/fsm/peer.rs Outdated Show resolved Hide resolved
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Apr 30, 2019

PTAL again @BusyJay @overvenus

@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Apr 30, 2019

/release

@zhangjinpeng1987

This comment has been minimized.

Copy link
Member

zhangjinpeng1987 commented May 4, 2019

@BusyJay @overvenus Any other problems?

@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Jun 24, 2019

conflict fixed, PTAL @BusyJay @overvenus

Connor1996 added 3 commits Jun 24, 2019
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@Connor1996 Connor1996 force-pushed the Connor1996:catch-up-log branch from 0ed41de to b15fcb9 Jun 27, 2019
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Jun 27, 2019

in maybe_append(), it will commit to the index. @BusyJay

pub fn maybe_append(
        &mut self,
        idx: u64,
        term: u64,
        committed: u64,
        ents: &[Entry],
    ) -> Option<u64> {
        let last_new_index = idx + ents.len() as u64;
        if self.match_term(idx, term) {
            let conflict_idx = self.find_conflict(ents);
            if conflict_idx == 0 {
            } else if conflict_idx <= self.committed {
                panic!(
                    "{} entry {} conflict with committed entry {}",
                    self.tag, conflict_idx, self.committed
                )
            } else {
                let offset = idx + 1;
                self.append(&ents[(conflict_idx - offset) as usize..]);
            }
            self.commit_to(cmp::min(committed, last_new_index));
            return Some(last_new_index);
        }
        None
    }
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Jun 28, 2019

PTAL again @BusyJay @overvenus

src/raftstore/store/fsm/peer.rs Show resolved Hide resolved
let mailbox = self.ctx.apply_router.mailbox(target).unwrap();
// Send CatchUpLogs back to destroy source apply delegate,
// then it will send `LogsUpToDate` to target apply delegate.
self.ctx.apply_router.schedule_task(

This comment has been minimized.

Copy link
@overvenus

overvenus Jun 28, 2019

Contributor

What if there are some cascaded merges? Can this region be destroyed?

This comment has been minimized.

Copy link
@Connor1996

Connor1996 Jul 8, 2019

Author Member

see https://github.com/tikv/tikv/pull/4595/files/b15fcb95aa35aecdddf632aecf108ed0336875b3#diff-641634cfd274be1752afcbf3c83d9200L2121, if there are some cascaded merges, it will return directly. And on_ready_prepare_merge is not called until all the cascaded merges are finished.

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Jul 25, 2019

@overvenus We can't send entries from target apply worker to source raftstore directly cause we need to get apply state in source apply worker which is attached with ApplyRes

@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Jul 25, 2019

PTAL again

clean
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Copy link
Contributor

overvenus left a comment

LGTM, could you write down the process order in the exec_commit_merges doc?

Connor1996 added 2 commits Jul 29, 2019
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
src/raftstore/store/fsm/apply.rs Outdated Show resolved Hide resolved
hicqu added 2 commits Jul 29, 2019
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Jul 30, 2019

/run-all-tests

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Jul 30, 2019

/run-all-tests

@hicqu
hicqu approved these changes Jul 30, 2019
@Connor1996 Connor1996 merged commit 4cbf21a into tikv:master Jul 30, 2019
2 of 3 checks passed
2 of 3 checks passed
idc-jenkins-ci-tikv/integration-common-test Jenkins job is running.
Details
DCO All commits are signed off!
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details
Connor1996 added a commit to Connor1996/tikv that referenced this pull request Jul 31, 2019
* catch up logs for merge through raft

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Connor1996 added a commit that referenced this pull request Aug 2, 2019
* raftstore, apply: catch up logs for merge through raft (#4595)

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
YangKeao added a commit to YangKeao/tikv that referenced this pull request Sep 5, 2019
sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019
* catch up logs for merge through raft

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.