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

fix: remark region as inactive on leader changed #2446

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
chore: by comment
  • Loading branch information
fengjiachun committed Sep 20, 2023
commit 37d18fb0161b0c1d917e6029dbfec3eb99b94afd
8 changes: 7 additions & 1 deletion src/meta-srv/src/procedure/region_failover.rs
Original file line number Diff line number Diff line change
@@ -270,6 +270,8 @@ trait State: Sync + Send + Debug {
fn status(&self) -> Status {
Status::executing(true)
}

fn remark_inactive_region_if_needed(&mut self) {}
}

/// The states transition of region failover procedure:
@@ -339,7 +341,11 @@ impl RegionFailoverProcedure {
}

fn from_json(json: &str, context: RegionFailoverContext) -> ProcedureResult<Self> {
let node: Node = serde_json::from_str(json).context(FromJsonSnafu)?;
let mut node: Node = serde_json::from_str(json).context(FromJsonSnafu)?;
// If the meta leader node dies during the execution of the procedure,
// the new leader node needs to remark the failed region as "inactive"
// to prevent it from renewing the lease.
node.state.remark_inactive_region_if_needed();
Ok(Self { node, context })
}
}
12 changes: 8 additions & 4 deletions src/meta-srv/src/procedure/region_failover/activate_region.rs
Original file line number Diff line number Diff line change
@@ -45,10 +45,10 @@ pub(super) struct ActivateRegion {
}

impl ActivateRegion {
pub(super) fn new(candidate: Peer, remark_inactive_region: bool) -> Self {
pub(super) fn new(candidate: Peer) -> Self {
Self {
candidate,
remark_inactive_region,
remark_inactive_region: false,
region_storage_path: None,
}
}
@@ -185,6 +185,10 @@ impl State for ActivateRegion {

self.handle_response(mailbox_receiver, failed_region).await
}

fn remark_inactive_region_if_needed(&mut self) {
self.remark_inactive_region = true;
}
}

#[cfg(test)]
@@ -203,7 +207,7 @@ mod tests {
let failed_region = env.failed_region(1).await;

let candidate = 2;
let mut state = ActivateRegion::new(Peer::new(candidate, ""), false);
let mut state = ActivateRegion::new(Peer::new(candidate, ""));
let mailbox_receiver = state
.send_open_region_message(&env.context, &failed_region, Duration::from_millis(100))
.await
@@ -274,7 +278,7 @@ mod tests {
let failed_region = env.failed_region(1).await;

let candidate = 2;
let mut state = ActivateRegion::new(Peer::new(candidate, ""), false);
let mut state = ActivateRegion::new(Peer::new(candidate, ""));
let mailbox_receiver = state
.send_open_region_message(&env.context, &failed_region, Duration::from_millis(100))
.await
Original file line number Diff line number Diff line change
@@ -97,7 +97,7 @@ impl DeactivateRegion {
.deregister_inactive_region(failed_region)
.await?;

Ok(Box::new(ActivateRegion::new(self.candidate.clone(), false)))
Ok(Box::new(ActivateRegion::new(self.candidate.clone())))
} else {
// Under rare circumstances would a Datanode fail to close a Region.
// So simply retry.
@@ -113,7 +113,7 @@ impl DeactivateRegion {
// the call and have disabled region lease renewal. Therefore, if a timeout error
// occurs, it can be concluded that the region has been closed. With this information,
// we can proceed confidently to the next step.
Ok(Box::new(ActivateRegion::new(self.candidate.clone(), true)))
Ok(Box::new(ActivateRegion::new(self.candidate.clone())))
}
Err(e) => Err(e),
}
@@ -146,7 +146,7 @@ impl State for DeactivateRegion {
);
// See the mailbox received timeout situation comments above.
self.wait_for_region_lease_expiry(ctx).await;
return Ok(Box::new(ActivateRegion::new(self.candidate.clone(), true)));
return Ok(Box::new(ActivateRegion::new(self.candidate.clone())));
}
Err(e) => return Err(e),
};
@@ -268,7 +268,7 @@ mod tests {
// Timeout or not, proceed to `ActivateRegion`.
assert_eq!(
format!("{next_state:?}"),
r#"ActivateRegion { candidate: Peer { id: 2, addr: "" }, remark_inactive_region: true, region_storage_path: None }"#
r#"ActivateRegion { candidate: Peer { id: 2, addr: "" }, remark_inactive_region: false, region_storage_path: None }"#
);
}
}