Skip to content

Commit

Permalink
chore: apply suggestions from CR
Browse files Browse the repository at this point in the history
  • Loading branch information
@WenyXu
WenyXu committed Sep 24, 2023
1 parent 3200038 commit 6f85d4a
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 30 deletions.
59 changes: 38 additions & 21 deletions src/common/meta/src/key.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use table_info::{TableInfoKey, TableInfoManager, TableInfoValue};
use table_name::{TableNameKey, TableNameManager, TableNameValue};

use self::catalog_name::{CatalogManager, CatalogNameKey, CatalogNameValue};
use self::datanode_table::UpdateDatanodeTableContext;
use self::schema_name::{SchemaManager, SchemaNameKey, SchemaNameValue};
use self::table_route::{TableRouteManager, TableRouteValue};
use crate::ddl::utils::region_storage_path;
Expand Down Expand Up @@ -448,14 +449,14 @@ impl TableMetadataManager {
pub async fn update_table_route(
&self,
table_id: TableId,
engine: &str,
region_storage_path: &str,
UpdateDatanodeTableContext {
engine,
region_storage_path,
region_options: current_region_options,
}: UpdateDatanodeTableContext<'_>,
current_table_route_value: TableRouteValue,
new_region_routes: Vec<RegionRoute>,
(current_region_options, new_region_options): (
&HashMap<String, String>,
&HashMap<String, String>,
),
new_region_options: &HashMap<String, String>,
) -> Result<()> {
// Updates the datanode table key value pairs.
let current_region_distribution =
Expand All @@ -464,11 +465,14 @@ impl TableMetadataManager {

let update_datanode_table_txn = self.datanode_table_manager().build_update_txn(
table_id,
engine,
region_storage_path,
UpdateDatanodeTableContext {
engine,
region_storage_path,
region_options: current_region_options,
},
current_region_distribution,
new_region_distribution,
(current_region_options, new_region_options),
new_region_options,
)?;

// Updates the table_route.
Expand Down Expand Up @@ -570,6 +574,7 @@ mod tests {

use super::datanode_table::DatanodeTableKey;
use crate::ddl::utils::region_storage_path;
use crate::key::datanode_table::UpdateDatanodeTableContext;
use crate::key::table_info::TableInfoValue;
use crate::key::table_name::TableNameKey;
use crate::key::table_route::TableRouteValue;
Expand Down Expand Up @@ -901,11 +906,14 @@ mod tests {
table_metadata_manager
.update_table_route(
table_id,
engine,
&region_storage_path,
UpdateDatanodeTableContext {
engine,
region_storage_path: &region_storage_path,
region_options: &HashMap::new(),
},
current_table_route_value.clone(),
new_region_routes.clone(),
(&HashMap::new(), &HashMap::new()),
&HashMap::new(),
)
.await
.unwrap();
Expand All @@ -915,11 +923,14 @@ mod tests {
table_metadata_manager
.update_table_route(
table_id,
engine,
&region_storage_path,
UpdateDatanodeTableContext {
engine,
region_storage_path: &region_storage_path,
region_options: &HashMap::new(),
},
current_table_route_value.clone(),
new_region_routes.clone(),
(&HashMap::new(), &HashMap::new()),
&HashMap::new(),
)
.await
.unwrap();
Expand All @@ -930,11 +941,14 @@ mod tests {
table_metadata_manager
.update_table_route(
table_id,
engine,
&region_storage_path,
UpdateDatanodeTableContext {
engine,
region_storage_path: &region_storage_path,
region_options: &HashMap::new(),
},
current_table_route_value.clone(),
new_region_routes.clone(),
(&HashMap::new(), &HashMap::new()),
&HashMap::new(),
)
.await
.unwrap();
Expand All @@ -951,11 +965,14 @@ mod tests {
assert!(table_metadata_manager
.update_table_route(
table_id,
engine,
&region_storage_path,
UpdateDatanodeTableContext {
engine,
region_storage_path: &region_storage_path,
region_options: &HashMap::new(),
},
wrong_table_route_value,
new_region_routes,
(&HashMap::new(), &HashMap::new()),
&HashMap::new(),
)
.await
.is_err());
Expand Down
17 changes: 11 additions & 6 deletions src/common/meta/src/key/datanode_table.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ use crate::rpc::store::RangeRequest;
use crate::rpc::KeyValue;
use crate::DatanodeId;

pub struct UpdateDatanodeTableContext<'a> {
pub engine: &'a str,
pub region_storage_path: &'a str,
pub region_options: &'a HashMap<String, String>,
}
pub struct DatanodeTableKey {
datanode_id: DatanodeId,
table_id: TableId,
Expand Down Expand Up @@ -189,14 +194,14 @@ impl DatanodeTableManager {
pub(crate) fn build_update_txn(
&self,
table_id: TableId,
engine: &str,
region_storage_path: &str,
UpdateDatanodeTableContext {
engine,
region_storage_path,
region_options: current_region_options,
}: UpdateDatanodeTableContext<'_>,
current_region_distribution: RegionDistribution,
new_region_distribution: RegionDistribution,
(current_region_options, new_region_options): (
&HashMap<String, String>,
&HashMap<String, String>,
),
new_region_options: &HashMap<String, String>,
) -> Result<Txn> {
let mut opts = Vec::new();

Expand Down
10 changes: 7 additions & 3 deletions src/meta-srv/src/procedure/region_failover/update_metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use std::collections::HashMap;

use async_trait::async_trait;
use common_meta::key::datanode_table::UpdateDatanodeTableContext;
use common_meta::key::table_route::TableRouteKey;
use common_meta::peer::Peer;
use common_meta::rpc::router::RegionRoute;
Expand Down Expand Up @@ -98,11 +99,14 @@ impl UpdateRegionMetadata {
ctx.table_metadata_manager
.update_table_route(
table_id,
engine,
&self.region_storage_path,
UpdateDatanodeTableContext {
engine,
region_storage_path: &self.region_storage_path,
region_options: &self.region_options,
},
table_route_value,
new_region_routes,
(&self.region_options, &self.region_options),
&self.region_options,
)
.await
.context(error::UpdateTableRouteSnafu)?;
Expand Down

0 comments on commit 6f85d4a

Please sign in to comment.