Skip to content

Commit efb51fb

Browse files
authored
refactor(meta): minor structural cleanups in process, control, admin, semaphore (#19470)
Fix 5 code quality items identified in the meta crate review: - process/kv_processor.rs: replace `unreachable!()` with `debug_assert!()` to avoid panics in release builds; `unreachable!` should be used only when the compiler cannot prove a code path is impossible. - process/process_meta_dir.rs: move the unused `v1_ent` suppression into the destructuring pattern (`_v1_ent`) instead of a separate `let _ = v1_ent` binding. - control/export_from_grpc.rs: replace `is_some() + unwrap()` with `if let Some(ref mut f)`, eliminating redundant borrows. - plugins/semaphore/semaphore.rs: initialize `seq_policy` directly with `GeneratorKey` instead of first setting `TimeBased` then overwriting it. - admin/v1/ctrl.rs: add a local `internal_err` helper (matching the one in `features.rs`) and replace the three verbose `map_err(|e| poem::Error::from_string(e.to_string(), INTERNAL_SERVER_ERROR))` chains with `.map_err(internal_err)`.
1 parent f489dfc commit efb51fb

File tree

5 files changed

+28
-37
lines changed

5 files changed

+28
-37
lines changed

src/meta/admin/src/v1/ctrl.rs

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,8 @@ impl<SP: SpawnApi> HttpService<SP> {
4444
meta_handle
4545
.handle_trigger_snapshot()
4646
.await
47-
.map_err(|e| {
48-
poem::Error::from_string(e.to_string(), StatusCode::INTERNAL_SERVER_ERROR)
49-
})?
50-
.map_err(|e| {
51-
poem::Error::from_string(e.to_string(), StatusCode::INTERNAL_SERVER_ERROR)
52-
})?;
47+
.map_err(internal_err)?
48+
.map_err(internal_err)?;
5349
Ok(Json(()).into_response())
5450
}
5551

@@ -60,9 +56,7 @@ impl<SP: SpawnApi> HttpService<SP> {
6056
let metrics = meta_handle
6157
.handle_raft_metrics()
6258
.await
63-
.map_err(|e| {
64-
poem::Error::from_string(e.to_string(), StatusCode::INTERNAL_SERVER_ERROR)
65-
})?
59+
.map_err(internal_err)?
6660
.borrow_watched()
6761
.clone();
6862

@@ -116,12 +110,8 @@ impl<SP: SpawnApi> HttpService<SP> {
116110
meta_handle
117111
.handle_trigger_transfer_leader(to)
118112
.await
119-
.map_err(|e| {
120-
poem::Error::from_string(e.to_string(), StatusCode::INTERNAL_SERVER_ERROR)
121-
})?
122-
.map_err(|e| {
123-
poem::Error::from_string(e.to_string(), StatusCode::INTERNAL_SERVER_ERROR)
124-
})?;
113+
.map_err(internal_err)?
114+
.map_err(internal_err)?;
125115

126116
Ok(Json(TransferLeaderResponse {
127117
from: id,
@@ -131,3 +121,7 @@ impl<SP: SpawnApi> HttpService<SP> {
131121
.into_response())
132122
}
133123
}
124+
125+
fn internal_err(e: impl std::fmt::Display) -> poem::Error {
126+
poem::Error::from_string(e.to_string(), StatusCode::INTERNAL_SERVER_ERROR)
127+
}

src/meta/control/src/export_from_grpc.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub async fn export_from_grpc(
9191

9292
let mut stream = exported.into_inner();
9393

94-
let file: Option<File> = if !save.is_empty() {
94+
let mut file: Option<File> = if !save.is_empty() {
9595
eprintln!(" To: File: {}", save);
9696
Some(File::create(&save)?)
9797
} else {
@@ -114,18 +114,16 @@ pub async fn export_from_grpc(
114114
}
115115
}
116116

117-
if file.as_ref().is_none() {
118-
println!("{}", line);
117+
if let Some(ref mut f) = file {
118+
f.write_all(format!("{}\n", line).as_bytes())?;
119119
} else {
120-
file.as_ref()
121-
.unwrap()
122-
.write_all(format!("{}\n", line).as_bytes())?;
120+
println!("{}", line);
123121
}
124122
}
125123
}
126124

127-
if file.as_ref().is_some() {
128-
file.as_ref().unwrap().sync_all()?;
125+
if let Some(ref f) = file {
126+
f.sync_all()?;
129127
}
130128

131129
Ok(())

src/meta/plugins/semaphore/src/semaphore.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ impl Semaphore {
164164
let uniq = UNIQ.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
165165

166166
let mut sem = Semaphore {
167-
prefix,
168-
seq_policy: SeqPolicy::TimeBased {
169-
timestamp: SystemTime::now().duration_since(UNIX_EPOCH).unwrap(),
167+
seq_policy: SeqPolicy::GeneratorKey {
168+
generator_key: Self::seq_generator_key_for(&prefix),
170169
},
170+
prefix,
171171
meta_client,
172172
permit_ttl,
173173
subscriber_task_handle: None,
@@ -176,11 +176,6 @@ impl Semaphore {
176176
uniq,
177177
};
178178

179-
// The default is using generator key
180-
sem.seq_policy = SeqPolicy::GeneratorKey {
181-
generator_key: sem.seq_generator_key(),
182-
};
183-
184179
sem.spawn_meta_event_subscriber(tx, capacity, cancel_rx)
185180
.await;
186181

@@ -326,7 +321,11 @@ impl Semaphore {
326321
///
327322
/// Update this key in the meta-service to obtain a new sequence number.
328323
fn seq_generator_key(&self) -> String {
329-
format!("{}/seq_generator", self.prefix)
324+
Self::seq_generator_key_for(&self.prefix)
325+
}
326+
327+
fn seq_generator_key_for(prefix: &str) -> String {
328+
format!("{}/seq_generator", prefix)
330329
}
331330

332331
/// The left-close right-open range for the semaphore ids.

src/meta/process/src/kv_processor.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,10 @@ where F: Fn(&str, Vec<u8>) -> Result<Vec<u8>, anyhow::Error>
187187
}
188188

189189
fn proc_condition(&self, c: TxnCondition) -> TxnCondition {
190-
if let Some(Target::Value(_)) = &c.target {
191-
unreachable!("we has never used value");
192-
}
190+
debug_assert!(
191+
!matches!(&c.target, Some(Target::Value(_))),
192+
"we have never used value"
193+
);
193194
c
194195
}
195196

src/meta/process/src/process_meta_dir.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ where F: Fn(RaftStoreEntry) -> Result<Option<RaftStoreEntry>, anyhow::Error> {
5151
converted.len()
5252
);
5353

54-
for (v1_ent, v2_ent) in converted {
55-
let _ = v1_ent;
54+
for (_v1_ent, v2_ent) in converted {
5655
let (k, v) = RaftStoreEntry::serialize(&v2_ent)?;
5756
tree.insert(k, v)?;
5857
}

0 commit comments

Comments
 (0)