diff --git a/src/meta-srv/src/region/lease_keeper.rs b/src/meta-srv/src/region/lease_keeper.rs index dc33abc6c9d0..5eb91feaf17b 100644 --- a/src/meta-srv/src/region/lease_keeper.rs +++ b/src/meta-srv/src/region/lease_keeper.rs @@ -12,20 +12,19 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub mod file; pub mod mito; pub mod utils; use std::collections::{HashMap, HashSet}; -use common_catalog::consts::{FILE_ENGINE, MITO2_ENGINE}; use common_meta::key::TableMetadataManagerRef; use snafu::ResultExt; -use store_api::region_engine::RegionRole; use store_api::storage::{RegionId, TableId}; +use self::mito::retain_active_regions; use crate::error::{self, Result}; +/// Region Lease Keeper removes any inactive [RegionRole::Leader] regions. pub struct RegionLeaseKeeper { table_metadata_manager: TableMetadataManagerRef, } @@ -39,46 +38,27 @@ impl RegionLeaseKeeper { } impl RegionLeaseKeeper { - /// Retains active mito regions(`datanode_regions`), returns inactive regions. - /// - /// **For mito regions:** - /// - /// - It removes a leader region if the `node_id` isn't the corresponding leader peer in `region_routes`. - /// - It removes a follower region if the `node_id` isn't one of the peers in `region_routes`. - /// - /// **For file regions:** - /// - /// - It removes a follower region if the `node_id` isn't one of the peers in `region_routes`. - /// - /// **Common behaviors:** + /// Retains active [RegionRole::Leader] regions, returns inactive regions. /// + /// - It grants new lease if the `datanode_id` is the corresponding leader peer in `region_routes`. + /// - It removes a leader region if the `datanode_id` isn't the corresponding leader peer in `region_routes`. + /// - Expected as [RegionRole::Follower] regions. + /// - Unexpected [RegionRole::Leader] regions. /// - It removes a region if the region's table metadata is not found. pub async fn retain_active_regions( &self, _cluster_id: u64, - node_id: u64, - engine: &str, - datanode_regions: &mut Vec<(RegionId, RegionRole)>, + datanode_id: u64, + datanode_regions: &mut Vec, ) -> Result> { let table_route_manager = self.table_metadata_manager.table_route_manager(); - let handler = match engine { - MITO2_ENGINE => mito::retain_active_regions, - FILE_ENGINE => file::retain_active_regions, - _ => { - return error::UnexpectedSnafu { - violated: format!("Unknown engine: {engine}"), - } - .fail() - } - }; - - let mut tables = HashMap::>::new(); + let mut tables = HashMap::>::new(); // Group by `table_id`. - for (region_id, role) in datanode_regions.iter() { + for region_id in datanode_regions.iter() { let table = tables.entry(region_id.table_id()).or_default(); - table.push((*region_id, *role)); + table.push(*region_id); } let table_ids = tables.keys().cloned().collect::>(); @@ -96,14 +76,14 @@ impl RegionLeaseKeeper { if let Some(metadata) = metadata_subset.get(table_id) { let region_routes = &metadata.region_routes; - inactive_regions.extend(handler(node_id, regions, region_routes)); + inactive_regions.extend(retain_active_regions(datanode_id, regions, region_routes)); } else { // Removes table if metadata is not found. - inactive_regions.extend(regions.drain(..).map(|(region_id, _)| region_id)); + inactive_regions.extend(regions.drain(..)); } } - datanode_regions.retain(|(region_id, _)| !inactive_regions.contains(region_id)); + datanode_regions.retain(|region_id| !inactive_regions.contains(region_id)); Ok(inactive_regions) } @@ -118,12 +98,10 @@ impl RegionLeaseKeeper { mod tests { use std::sync::Arc; - use common_catalog::consts::MITO2_ENGINE; use common_meta::key::test_utils::new_test_table_info; use common_meta::key::TableMetadataManager; use common_meta::peer::Peer; use common_meta::rpc::router::{Region, RegionRoute}; - use store_api::region_engine::RegionRole; use store_api::storage::RegionId; use super::RegionLeaseKeeper; @@ -140,17 +118,16 @@ mod tests { #[tokio::test] async fn test_empty_table_routes() { - let node_id = 1; - let engine = MITO2_ENGINE; + let datanode_id = 1; let region_number = 1u32; let region_id = RegionId::from_u64(region_number as u64); let keeper = new_test_keeper(); - let mut datanode_regions = vec![(region_id, RegionRole::Leader)]; + let mut datanode_regions = vec![region_id]; let removed = keeper - .retain_active_regions(0, node_id, engine, &mut datanode_regions) + .retain_active_regions(0, datanode_id, &mut datanode_regions) .await .unwrap(); @@ -161,12 +138,11 @@ mod tests { #[tokio::test] async fn test_retain_active_regions_simple() { - let node_id = 1; - let engine = MITO2_ENGINE; + let datanode_id = 1; let region_number = 1u32; let table_id = 10; let region_id = RegionId::new(table_id, region_number); - let peer = Peer::empty(node_id); + let peer = Peer::empty(datanode_id); let table_info = new_test_table_info(table_id, vec![region_number]).into(); let region_routes = vec![RegionRoute { @@ -183,22 +159,22 @@ mod tests { .await .unwrap(); - // `inactive_regions` should be vec![region_id]. - let mut datanode_regions = vec![(region_id, RegionRole::Leader)]; + // `inactive_regions` should be empty. + let mut datanode_regions = vec![region_id]; let inactive_regions = keeper - .retain_active_regions(0, node_id, engine, &mut datanode_regions) + .retain_active_regions(0, datanode_id, &mut datanode_regions) .await .unwrap(); assert!(inactive_regions.is_empty()); assert_eq!(datanode_regions.len(), 1); - // `inactive_regions` should be empty, because the `region_id` is a potential leader. - let mut datanode_regions = vec![(region_id, RegionRole::Follower)]; + // `inactive_regions` should be empty. + let mut datanode_regions = vec![]; let inactive_regions = keeper - .retain_active_regions(0, node_id, engine, &mut datanode_regions) + .retain_active_regions(0, datanode_id, &mut datanode_regions) .await .unwrap(); @@ -207,16 +183,15 @@ mod tests { #[tokio::test] async fn test_retain_active_regions_2() { - let node_id = 1; - let engine = MITO2_ENGINE; + let datanode_id = 1; let region_number = 1u32; let table_id = 10; let region_id = RegionId::new(table_id, region_number); let another_region_id = RegionId::new(table_id, region_number + 1); let unknown_region_id = RegionId::new(table_id + 1, region_number); - let peer = Peer::empty(node_id); - let another_peer = Peer::empty(node_id + 1); + let peer = Peer::empty(datanode_id); + let another_peer = Peer::empty(datanode_id + 1); let table_info = new_test_table_info(table_id, vec![region_number, region_number + 1]).into(); @@ -242,14 +217,12 @@ mod tests { .await .unwrap(); + // Unexpected Leader region. // `inactive_regions` should be vec![unknown_region_id]. - let mut datanode_regions = vec![ - (region_id, RegionRole::Leader), - (unknown_region_id, RegionRole::Follower), - ]; + let mut datanode_regions = vec![region_id, unknown_region_id]; let inactive_regions = keeper - .retain_active_regions(0, node_id, engine, &mut datanode_regions) + .retain_active_regions(0, datanode_id, &mut datanode_regions) .await .unwrap(); @@ -257,34 +230,17 @@ mod tests { assert!(inactive_regions.contains(&unknown_region_id)); assert_eq!(datanode_regions.len(), 1); + // Expected as Follower region. // `inactive_regions` should be vec![another_region_id], because the `another_region_id` is a active region of `another_peer`. - let mut datanode_regions = vec![ - (region_id, RegionRole::Follower), - (another_region_id, RegionRole::Follower), - ]; - - let inactive_regions = keeper - .retain_active_regions(0, node_id, engine, &mut datanode_regions) - .await - .unwrap(); - - assert_eq!(inactive_regions.len(), 1); - assert!(inactive_regions.contains(&another_region_id)); - assert_eq!(datanode_regions.len(), 1); - - // `inactive_regions` should be vec![another_region_id], because the `another_region_id` is a active region of `another_peer`. - let mut datanode_regions = vec![ - (region_id, RegionRole::Follower), - (another_region_id, RegionRole::Leader), - ]; + let mut datanode_regions = vec![another_region_id]; let inactive_regions = keeper - .retain_active_regions(0, node_id, engine, &mut datanode_regions) + .retain_active_regions(0, datanode_id, &mut datanode_regions) .await .unwrap(); assert_eq!(inactive_regions.len(), 1); assert!(inactive_regions.contains(&another_region_id)); - assert_eq!(datanode_regions.len(), 1); + assert!(datanode_regions.is_empty()); } } diff --git a/src/meta-srv/src/region/lease_keeper/file.rs b/src/meta-srv/src/region/lease_keeper/file.rs deleted file mode 100644 index 903ad968c6e6..000000000000 --- a/src/meta-srv/src/region/lease_keeper/file.rs +++ /dev/null @@ -1,166 +0,0 @@ -// Copyright 2023 Greptime Team -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use std::collections::HashSet; - -use common_meta::rpc::router::{convert_to_region_peers_map, RegionRoute}; -use store_api::region_engine::RegionRole; -use store_api::storage::RegionId; - -use super::utils::inactive_follower_regions; - -/// Retains active mito regions(`datanode_regions`), returns inactive regions. -/// -/// It removes a region if the `node_id` isn't one of the peers in `region_routes`. -pub fn retain_active_regions( - node_id: u64, - datanode_regions: &mut Vec<(RegionId, RegionRole)>, - region_routes: &[RegionRoute], -) -> HashSet { - let region_peers_map = convert_to_region_peers_map(region_routes); - - let inactive_region_ids = datanode_regions - .clone() - .into_iter() - .filter_map(|(region_id, _)| { - inactive_follower_regions(node_id, region_id, ®ion_peers_map) - }) - .collect::>(); - - datanode_regions.retain(|(region_id, _)| !inactive_region_ids.contains(region_id)); - - inactive_region_ids -} - -#[cfg(test)] -mod tests { - - use common_meta::peer::Peer; - use common_meta::rpc::router::{Region, RegionRoute}; - use store_api::region_engine::RegionRole; - use store_api::storage::RegionId; - - use crate::region::lease_keeper::file::retain_active_regions; - - #[test] - fn test_retain_active_regions() { - let node_id = 1u64; - let region_number = 1u32; - let region_id = RegionId::from_u64(region_number as u64); - let peer = Peer::empty(node_id); - - let another_region_id = RegionId::from_u64(region_number as u64 + 1); - let another_peer = Peer::empty(node_id + 1); - - let datanode_regions = vec![(region_id, RegionRole::Follower)]; - let region_routes = vec![RegionRoute { - region: Region::new_test(region_id), - leader_peer: Some(peer.clone()), - ..Default::default() - }]; - - // `inactive_regions` should be empty, `region_id` is a active region of the `peer` - let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions.clone(), ®ion_routes); - - assert!(inactive_regions.is_empty()); - - let region_routes = vec![RegionRoute { - region: Region::new_test(region_id), - leader_peer: None, - follower_peers: vec![peer.clone()], - }]; - - // `inactive_regions` should be empty, `region_id` is a active region of the `peer` - let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions.clone(), ®ion_routes); - - assert!(inactive_regions.is_empty()); - - let region_routes = vec![ - RegionRoute { - region: Region::new_test(region_id), - leader_peer: Some(peer.clone()), - ..Default::default() - }, - RegionRoute { - region: Region::new_test(another_region_id), - leader_peer: None, - follower_peers: vec![peer.clone()], - }, - ]; - - // `inactive_regions` should be empty, `region_id` is a active region of the `peer` - let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions.clone(), ®ion_routes); - - assert!(inactive_regions.is_empty()); - - // `inactive_regions` should be vec[`region_id`,`another_region_id`], - // both regions are the active region of the `another_peer`. - let region_routes = vec![ - RegionRoute { - region: Region::new_test(region_id), - leader_peer: Some(another_peer.clone()), - ..Default::default() - }, - RegionRoute { - region: Region::new_test(another_region_id), - leader_peer: None, - follower_peers: vec![another_peer.clone()], - }, - ]; - - let mut datanode_regions = vec![ - (region_id, RegionRole::Follower), - (another_region_id, RegionRole::Follower), - ]; - - let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions, ®ion_routes); - - assert_eq!(inactive_regions.len(), 2); - assert!(inactive_regions.contains(®ion_id)); - assert!(inactive_regions.contains(&another_region_id)); - assert!(datanode_regions.is_empty()); - - // `inactive_regions` should be vec[`another_region_id`], - // `another_region_id` regions are the active region of the `another_peer`. - let region_routes = vec![ - RegionRoute { - region: Region::new_test(region_id), - leader_peer: Some(peer.clone()), - ..Default::default() - }, - RegionRoute { - region: Region::new_test(another_region_id), - leader_peer: None, - follower_peers: vec![another_peer], - }, - ]; - - let mut datanode_regions = vec![ - (region_id, RegionRole::Follower), - (another_region_id, RegionRole::Follower), - ]; - - let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions, ®ion_routes); - - assert_eq!(inactive_regions.len(), 1); - assert!(inactive_regions.contains(&another_region_id)); - assert_eq!(datanode_regions.len(), 1); - assert!(datanode_regions.contains(&(region_id, RegionRole::Follower))); - } -} diff --git a/src/meta-srv/src/region/lease_keeper/mito.rs b/src/meta-srv/src/region/lease_keeper/mito.rs index b131f181c00e..497ea8726704 100644 --- a/src/meta-srv/src/region/lease_keeper/mito.rs +++ b/src/meta-srv/src/region/lease_keeper/mito.rs @@ -14,39 +14,30 @@ use std::collections::HashSet; -use common_meta::rpc::router::{ - convert_to_region_leader_map, convert_to_region_peers_map, RegionRoute, -}; -use store_api::region_engine::RegionRole; +use common_meta::rpc::router::{convert_to_region_leader_map, RegionRoute}; use store_api::storage::RegionId; -use crate::region::lease_keeper::utils::{inactive_follower_regions, inactive_leader_regions}; +use crate::region::lease_keeper::utils::inactive_leader_regions; -/// Retains active mito regions(`datanode_regions`), returns inactive regions. +/// Retains active [RegionRole::Leader](store_api::region_engine::RegionRole::Leader) regions(`datanode_regions`), returns inactive regions. /// -/// It removes a leader region if the `node_id` isn't the corresponding leader peer in `region_routes`. -/// -/// It removes a follower region if the `node_id` isn't one of the peers in `region_routes`. +/// It removes a leader region if the `datanode_id` isn't the corresponding leader peer in `region_routes`. +/// - Expected as [RegionRole::Follower](store_api::region_engine::RegionRole::Follower) regions. +/// - Unexpected [RegionRole::Leader](store_api::region_engine::RegionRole::Leader) regions. pub fn retain_active_regions( - node_id: u64, - datanode_regions: &mut Vec<(RegionId, RegionRole)>, + datanode_id: u64, + datanode_regions: &mut Vec, region_routes: &[RegionRoute], ) -> HashSet { let region_leader_map = convert_to_region_leader_map(region_routes); - let region_peers_map = convert_to_region_peers_map(region_routes); let inactive_region_ids = datanode_regions .clone() .into_iter() - .filter_map(|(region_id, role)| match role { - RegionRole::Follower => { - inactive_follower_regions(node_id, region_id, ®ion_peers_map) - } - RegionRole::Leader => inactive_leader_regions(node_id, region_id, ®ion_leader_map), - }) + .filter_map(|region_id| inactive_leader_regions(datanode_id, region_id, ®ion_leader_map)) .collect::>(); - datanode_regions.retain(|(region_id, _)| !inactive_region_ids.contains(region_id)); + datanode_regions.retain(|region_id| !inactive_region_ids.contains(region_id)); inactive_region_ids } @@ -56,36 +47,38 @@ mod tests { use common_meta::peer::Peer; use common_meta::rpc::router::{Region, RegionRoute}; - use store_api::region_engine::RegionRole; use store_api::storage::RegionId; use crate::region::lease_keeper::mito::retain_active_regions; #[test] fn test_retain_active_regions() { - let node_id = 1u64; + let datanode_id = 1u64; let region_number = 1u32; let region_id = RegionId::from_u64(region_number as u64); - let peer = Peer::empty(node_id); - let another_peer = Peer::empty(node_id + 1); + let peer = Peer::empty(datanode_id); + let another_peer = Peer::empty(datanode_id + 1); - let datanode_regions = vec![(region_id, RegionRole::Leader)]; + let datanode_regions = vec![region_id]; let region_routes = vec![RegionRoute { region: Region::new_test(region_id), leader_peer: Some(peer.clone()), ..Default::default() }]; + // Grants lease. // `inactive_regions` should be empty, `region_id` is a active leader region of the `peer` let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions.clone(), ®ion_routes); + retain_active_regions(datanode_id, &mut datanode_regions.clone(), ®ion_routes); assert!(inactive_regions.is_empty()); let mut retained_active_regions = datanode_regions.clone(); + // Unexpected Leader region. // `inactive_regions` should be vec![`region_id`]; - let inactive_regions = retain_active_regions(node_id, &mut retained_active_regions, &[]); + let inactive_regions = + retain_active_regions(datanode_id, &mut retained_active_regions, &[]); assert_eq!(inactive_regions.len(), 1); assert!(inactive_regions.contains(®ion_id)); @@ -99,60 +92,13 @@ mod tests { let mut retained_active_regions = datanode_regions.clone(); + // Expected as Follower region. // `inactive_regions` should be vec![`region_id`], `region_id` is RegionRole::Leader. let inactive_regions = - retain_active_regions(node_id, &mut retained_active_regions, ®ion_routes); + retain_active_regions(datanode_id, &mut retained_active_regions, ®ion_routes); assert_eq!(inactive_regions.len(), 1); assert!(inactive_regions.contains(®ion_id)); assert!(retained_active_regions.is_empty()); - - // `inactive_regions` should be empty, `region_id` is a active follower region of the `peer`. - let datanode_regions = vec![(region_id, RegionRole::Follower)]; - let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions.clone(), ®ion_routes); - assert!(inactive_regions.is_empty()); - - let another_region_id = RegionId::from_u64(region_number as u64 + 1); - - let region_routes = vec![ - RegionRoute { - region: Region::new_test(region_id), - leader_peer: Some(peer.clone()), - ..Default::default() - }, - RegionRoute { - region: Region::new_test(another_region_id), - leader_peer: None, - follower_peers: vec![peer], - }, - ]; - - let datanode_regions = vec![ - (region_id, RegionRole::Leader), - (another_region_id, RegionRole::Follower), - ]; - - let inactive_regions = - retain_active_regions(node_id, &mut datanode_regions.clone(), ®ion_routes); - - assert!(inactive_regions.is_empty()); - - // `inactive_regions` should be vec![another_region_id]. - let datanode_regions = vec![ - (another_region_id, RegionRole::Leader), - (region_id, RegionRole::Follower), - ]; - - let mut retained_active_regions = datanode_regions.clone(); - - let inactive_regions = - retain_active_regions(node_id, &mut retained_active_regions, ®ion_routes); - - assert_eq!(inactive_regions.len(), 1); - assert!(inactive_regions.contains(&another_region_id)); - - assert_eq!(retained_active_regions.len(), 1); - assert!(retained_active_regions.contains(&(region_id, RegionRole::Follower))); } } diff --git a/src/meta-srv/src/region/lease_keeper/utils.rs b/src/meta-srv/src/region/lease_keeper/utils.rs index 4cec956a6947..7acd0150c797 100644 --- a/src/meta-srv/src/region/lease_keeper/utils.rs +++ b/src/meta-srv/src/region/lease_keeper/utils.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use common_meta::peer::Peer; use store_api::storage::RegionId; @@ -37,25 +37,6 @@ pub fn inactive_leader_regions( } } -/// Returns Some(region_id) if it's a inactive follower region. -/// -/// It removes a region if the `node_id` isn't one of the peers in `region_routes`. -pub fn inactive_follower_regions( - node_id: u64, - region_id: RegionId, - region_peer_map: &HashMap>, -) -> Option { - if let Some(peers) = region_peer_map.get(®ion_id.region_number()) { - if peers.contains(&node_id) { - None - } else { - Some(region_id) - } - } else { - Some(region_id) - } -} - #[cfg(test)] mod tests { @@ -64,66 +45,34 @@ mod tests { use super::*; - #[test] - fn test_inactive_follower_regions() { - let node_id = 1u64; - let region_number = 1u32; - let region_id = RegionId::from_u64(region_number as u64); - let peer = Peer::empty(node_id); - - let region_peers_map = [(region_number, HashSet::from([node_id]))].into(); - - // Should be None, `region_id` is a active region of `peer`. - assert_eq!( - None, - inactive_follower_regions(peer.id, region_id, ®ion_peers_map) - ); - - // Should be Some(`region_id`), region_followers_map is empty. - assert_eq!( - Some(region_id), - inactive_follower_regions(peer.id, region_id, &Default::default()) - ); - - let another_peer = Peer::empty(node_id + 1); - - let region_peers_map = [(region_number, HashSet::from([peer.id, another_peer.id]))].into(); - - // Should be None, `region_id` is a active region of `another_peer`. - assert_eq!( - None, - inactive_follower_regions(node_id, region_id, ®ion_peers_map) - ); - } - #[test] fn test_inactive_leader_regions() { - let node_id = 1u64; + let datanode_id = 1u64; let region_number = 1u32; let region_id = RegionId::from_u64(region_number as u64); - let peer = Peer::empty(node_id); + let peer = Peer::empty(datanode_id); let region_leader_map = [(region_number, &peer)].into(); // Should be None, `region_id` is a active region of `peer`. assert_eq!( None, - inactive_leader_regions(node_id, region_id, ®ion_leader_map) + inactive_leader_regions(datanode_id, region_id, ®ion_leader_map) ); // Should be Some(`region_id`), the inactive_leader_regions is empty. assert_eq!( Some(region_id), - inactive_leader_regions(node_id, region_id, &Default::default()) + inactive_leader_regions(datanode_id, region_id, &Default::default()) ); - let another_peer = Peer::empty(node_id + 1); + let another_peer = Peer::empty(datanode_id + 1); let region_leader_map = [(region_number, &another_peer)].into(); // Should be Some(`region_id`), `region_id` is active region of `another_peer`. assert_eq!( Some(region_id), - inactive_leader_regions(node_id, region_id, ®ion_leader_map) + inactive_leader_regions(datanode_id, region_id, ®ion_leader_map) ); } }