Skip to content

Commit

Permalink
chore: deny unused results (GreptimeTeam#1825)
Browse files Browse the repository at this point in the history
* chore: deny unused results

* rebase
  • Loading branch information
MichaelScofield authored and paomian committed Oct 19, 2023
1 parent baa3bc9 commit f4e200e
Show file tree
Hide file tree
Showing 247 changed files with 1,209 additions and 1,171 deletions.
1 change: 1 addition & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ rustflags = [
"-Wclippy::print_stderr",
"-Wclippy::implicit_clone",
"-Aclippy::items_after_test_module",
"-Wunused_results",
]
31 changes: 15 additions & 16 deletions benchmarks/src/bin/nyc-taxi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ async fn write_data(
};

let now = Instant::now();
db.insert(requests).await.unwrap();
let _ = db.insert(requests).await.unwrap();
let elapsed = now.elapsed();
total_rpc_elapsed_ms += elapsed.as_millis();
progress_bar.inc(row_count as _);
Expand Down Expand Up @@ -377,19 +377,16 @@ fn create_table_expr() -> CreateTableExpr {
}

fn query_set() -> HashMap<String, String> {
let mut ret = HashMap::new();

ret.insert(
"count_all".to_string(),
format!("SELECT COUNT(*) FROM {TABLE_NAME};"),
);

ret.insert(
"fare_amt_by_passenger".to_string(),
format!("SELECT passenger_count, MIN(fare_amount), MAX(fare_amount), SUM(fare_amount) FROM {TABLE_NAME} GROUP BY passenger_count")
);

ret
HashMap::from([
(
"count_all".to_string(),
format!("SELECT COUNT(*) FROM {TABLE_NAME};"),
),
(
"fare_amt_by_passenger".to_string(),
format!("SELECT passenger_count, MIN(fare_amount), MAX(fare_amount), SUM(fare_amount) FROM {TABLE_NAME} GROUP BY passenger_count"),
)
])
}

async fn do_write(args: &Args, db: &Database) {
Expand All @@ -414,7 +411,8 @@ async fn do_write(args: &Args, db: &Database) {
let db = db.clone();
let mpb = multi_progress_bar.clone();
let pb_style = progress_bar_style.clone();
write_jobs.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
let _ = write_jobs
.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
}
}
while write_jobs.join_next().await.is_some() {
Expand All @@ -423,7 +421,8 @@ async fn do_write(args: &Args, db: &Database) {
let db = db.clone();
let mpb = multi_progress_bar.clone();
let pb_style = progress_bar_style.clone();
write_jobs.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
let _ = write_jobs
.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/catalog/src/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,6 @@ mod tests {
#[test]
fn test_table_global_value_compatibility() {
let s = r#"{"node_id":1,"regions_id_map":{"1":[0]},"table_info":{"ident":{"table_id":1098,"version":1},"name":"container_cpu_limit","desc":"Created on insertion","catalog_name":"greptime","schema_name":"dd","meta":{"schema":{"column_schemas":[{"name":"container_id","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"container_name","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"docker_image","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"host","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"image_name","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"image_tag","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"interval","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"runtime","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"short_image","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"type","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"dd_value","data_type":{"Float64":{}},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"ts","data_type":{"Timestamp":{"Millisecond":null}},"is_nullable":false,"is_time_index":true,"default_constraint":null,"metadata":{"greptime:time_index":"true"}},{"name":"git.repository_url","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}}],"timestamp_index":11,"version":1},"primary_key_indices":[0,1,2,3,4,5,6,7,8,9,12],"value_indices":[10,11],"engine":"mito","next_column_id":12,"region_numbers":[],"engine_options":{},"options":{},"created_on":"1970-01-01T00:00:00Z"},"table_type":"Base"}}"#;
TableGlobalValue::parse(s).unwrap();
assert!(TableGlobalValue::parse(s).is_ok());
}
}
2 changes: 1 addition & 1 deletion src/catalog/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub(crate) async fn handle_system_table_request<'a, M: CatalogManager>(
table_name,
),
})?;
manager
let _ = manager
.register_table(RegisterTableRequest {
catalog: catalog_name.clone(),
schema: schema_name.clone(),
Expand Down
33 changes: 20 additions & 13 deletions src/catalog/src/local/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ impl LocalCatalogManager {

async fn init_system_catalog(&self) -> Result<()> {
// register SystemCatalogTable
self.catalogs
let _ = self
.catalogs
.register_catalog_sync(SYSTEM_CATALOG_NAME.to_string())?;
self.catalogs.register_schema_sync(RegisterSchemaRequest {
let _ = self.catalogs.register_schema_sync(RegisterSchemaRequest {
catalog: SYSTEM_CATALOG_NAME.to_string(),
schema: INFORMATION_SCHEMA_NAME.to_string(),
})?;
Expand All @@ -131,12 +132,13 @@ impl LocalCatalogManager {
table_id: SYSTEM_CATALOG_TABLE_ID,
table: self.system.information_schema.system.clone(),
};
self.catalogs.register_table(register_table_req).await?;
let _ = self.catalogs.register_table(register_table_req).await?;

// register default catalog and default schema
self.catalogs
let _ = self
.catalogs
.register_catalog_sync(DEFAULT_CATALOG_NAME.to_string())?;
self.catalogs.register_schema_sync(RegisterSchemaRequest {
let _ = self.catalogs.register_schema_sync(RegisterSchemaRequest {
catalog: DEFAULT_CATALOG_NAME.to_string(),
schema: DEFAULT_SCHEMA_NAME.to_string(),
})?;
Expand All @@ -151,7 +153,8 @@ impl LocalCatalogManager {
table: numbers_table,
};

self.catalogs
let _ = self
.catalogs
.register_table(register_number_table_req)
.await?;

Expand Down Expand Up @@ -226,7 +229,8 @@ impl LocalCatalogManager {
for entry in entries {
match entry {
Entry::Catalog(c) => {
self.catalogs
let _ = self
.catalogs
.register_catalog_if_absent(c.catalog_name.clone());
info!("Register catalog: {}", c.catalog_name);
}
Expand All @@ -235,7 +239,7 @@ impl LocalCatalogManager {
catalog: s.catalog_name.clone(),
schema: s.schema_name.clone(),
};
self.catalogs.register_schema_sync(req)?;
let _ = self.catalogs.register_schema_sync(req)?;
info!("Registered schema: {:?}", s);
}
Entry::Table(t) => {
Expand Down Expand Up @@ -297,7 +301,7 @@ impl LocalCatalogManager {
table_id: t.table_id,
table: table_ref,
};
self.catalogs.register_table(register_request).await?;
let _ = self.catalogs.register_table(register_request).await?;

Ok(())
}
Expand Down Expand Up @@ -389,8 +393,9 @@ impl CatalogManager for LocalCatalogManager {
let engine = request.table.table_info().meta.engine.to_string();
let table_name = request.table_name.clone();
let table_id = request.table_id;
self.catalogs.register_table(request).await?;
self.system
let _ = self.catalogs.register_table(request).await?;
let _ = self
.system
.register_table(
catalog_name.clone(),
schema_name.clone(),
Expand Down Expand Up @@ -438,7 +443,8 @@ impl CatalogManager for LocalCatalogManager {

let engine = old_table.table_info().meta.engine.to_string();
// rename table in system catalog
self.system
let _ = self
.system
.register_table(
catalog_name.clone(),
schema_name.clone(),
Expand Down Expand Up @@ -499,7 +505,8 @@ impl CatalogManager for LocalCatalogManager {
schema: schema_name,
}
);
self.system
let _ = self
.system
.register_schema(request.catalog.clone(), schema_name.clone())
.await?;
self.catalogs.register_schema_sync(request)
Expand Down
35 changes: 19 additions & 16 deletions src/catalog/src/local/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ impl Default for MemoryCatalogManager {
catalogs: Default::default(),
};

let mut catalog = HashMap::with_capacity(1);
catalog.insert(DEFAULT_SCHEMA_NAME.to_string(), HashMap::new());
manager
let catalog = HashMap::from([(DEFAULT_SCHEMA_NAME.to_string(), HashMap::new())]);
let _ = manager
.catalogs
.write()
.unwrap()
Expand Down Expand Up @@ -115,7 +114,7 @@ impl CatalogManager for MemoryCatalogManager {
}

let table = schema.remove(&request.table_name).unwrap();
schema.insert(request.new_table_name, table);
let _ = schema.insert(request.new_table_name, table);

Ok(true)
}
Expand Down Expand Up @@ -144,9 +143,11 @@ impl CatalogManager for MemoryCatalogManager {
}

async fn register_schema(&self, request: RegisterSchemaRequest) -> Result<bool> {
self.register_schema_sync(request)?;
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0);
Ok(true)
let registered = self.register_schema_sync(request)?;
if registered {
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0);
}
Ok(registered)
}

async fn register_system_table(&self, _request: RegisterSystemTableRequest) -> Result<()> {
Expand Down Expand Up @@ -234,9 +235,11 @@ impl CatalogManager for MemoryCatalogManager {
}

async fn register_catalog(&self, name: String) -> Result<bool> {
self.register_catalog_sync(name)?;
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0);
Ok(true)
let registered = self.register_catalog_sync(name)?;
if registered {
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0);
}
Ok(registered)
}

fn as_any(&self) -> &dyn Any {
Expand All @@ -252,7 +255,7 @@ impl MemoryCatalogManager {
match entry {
Entry::Occupied(_) => true,
Entry::Vacant(v) => {
v.insert(HashMap::new());
let _ = v.insert(HashMap::new());
false
}
}
Expand All @@ -273,7 +276,7 @@ impl MemoryCatalogManager {
if catalog.contains_key(&request.schema) {
return Ok(false);
}
catalog.insert(request.schema, HashMap::new());
let _ = catalog.insert(request.schema, HashMap::new());
Ok(true)
}

Expand Down Expand Up @@ -310,7 +313,7 @@ impl MemoryCatalogManager {
table_id: table.table_info().ident.table_id,
table,
};
manager.register_table_sync(request).unwrap();
let _ = manager.register_table_sync(request).unwrap();
manager
}
}
Expand Down Expand Up @@ -341,7 +344,7 @@ mod tests {
table: Arc::new(NumbersTable::default()),
};

catalog_list.register_table(register_request).await.unwrap();
assert!(catalog_list.register_table(register_request).await.is_ok());
let table = catalog_list
.table(
DEFAULT_CATALOG_NAME,
Expand Down Expand Up @@ -390,7 +393,7 @@ mod tests {
new_table_name: new_table_name.to_string(),
table_id,
};
catalog.rename_table(rename_request).await.unwrap();
assert!(catalog.rename_table(rename_request).await.is_ok());

// test old table name not exist
assert!(!catalog
Expand Down Expand Up @@ -492,7 +495,7 @@ mod tests {
table_id: 2333,
table: Arc::new(NumbersTable::default()),
};
catalog.register_table(register_table_req).await.unwrap();
assert!(catalog.register_table(register_table_req).await.is_ok());
assert!(catalog
.table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name)
.await
Expand Down
2 changes: 1 addition & 1 deletion src/catalog/src/remote/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ impl KvBackend for MetaKvBackend {

async fn move_value(&self, from_key: &[u8], to_key: &[u8]) -> Result<()> {
let req = MoveValueRequest::new(from_key, to_key);
self.client.move_value(req).await.context(MetaSrvSnafu)?;
let _ = self.client.move_value(req).await.context(MetaSrvSnafu)?;
Ok(())
}

Expand Down
26 changes: 14 additions & 12 deletions src/catalog/src/remote/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl RemoteCatalogManager {
joins.push(self.initiate_schemas(node_id, backend, engine_manager, catalog_name));
}

futures::future::try_join_all(joins).await?;
let _ = futures::future::try_join_all(joins).await?;

Ok(())
}
Expand Down Expand Up @@ -623,13 +623,14 @@ impl CatalogManager for RemoteCatalogManager {
self.check_catalog_schema_exist(&catalog_name, &schema_name)
.await?;

self.register_table(
catalog_name.clone(),
schema_name.clone(),
request.table_name,
request.table.clone(),
)
.await?;
let _ = self
.register_table(
catalog_name.clone(),
schema_name.clone(),
request.table_name,
request.table.clone(),
)
.await?;

let table_info = request.table.table_info();
let table_ident = TableIdent {
Expand Down Expand Up @@ -680,7 +681,8 @@ impl CatalogManager for RemoteCatalogManager {
table_id: table_info.ident.table_id,
engine: table_info.meta.engine.clone(),
};
self.region_alive_keepers
let _ = self
.region_alive_keepers
.deregister_table(&table_ident)
.await;
}
Expand Down Expand Up @@ -846,7 +848,7 @@ impl CatalogManager for RemoteCatalogManager {
let catalog_key = String::from_utf8_lossy(&catalog.0);

if let Ok(key) = CatalogKey::parse(&catalog_key) {
catalogs.insert(key.catalog_name);
let _ = catalogs.insert(key.catalog_name);
}
}
}
Expand All @@ -865,7 +867,7 @@ impl CatalogManager for RemoteCatalogManager {
let schema_key = String::from_utf8_lossy(&schema.0);

if let Ok(key) = SchemaKey::parse(&schema_key) {
schemas.insert(key.schema_name);
let _ = schemas.insert(key.schema_name);
}
}
}
Expand All @@ -886,7 +888,7 @@ impl CatalogManager for RemoteCatalogManager {
let table_key = String::from_utf8_lossy(&table.0);

if let Ok(key) = TableRegionalKey::parse(&table_key) {
tables.insert(key.table_name);
let _ = tables.insert(key.table_name);
}
}
}
Expand Down
Loading

0 comments on commit f4e200e

Please sign in to comment.