Skip to content

Commit ac3fc23

Browse files
authored
fix: remove potential panics and improve robustness (#14)
- Fix shard_manager: replace unwrap() on shard_boundaries with if-let to gracefully handle assignments without boundaries instead of panicking - Add Display impl for Notification enum for better logging Signed-off-by: mattisonchao <mattisonchao@gmail.com>
1 parent 817782f commit ac3fc23

File tree

2 files changed

+39
-11
lines changed

2 files changed

+39
-11
lines changed

liboxia-native/src/client.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use crate::shard_manager::{Node, ShardManager, ShardManagerOptions};
2020
use crate::write_stream_manager::WriteStreamManager;
2121
use dashmap::DashMap;
2222
use std::cmp::Ordering;
23+
use std::fmt;
2324
use std::sync::Arc;
2425
use std::time::Duration;
2526
use tokio::sync::mpsc::Receiver;
@@ -190,6 +191,30 @@ pub enum Notification {
190191
Unknown(),
191192
}
192193

194+
impl fmt::Display for Notification {
195+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
196+
match self {
197+
Notification::KeyCreated(c) => write!(
198+
f,
199+
"KeyCreated(key={}, version_id={:?})",
200+
c.key, c.version_id
201+
),
202+
Notification::KeyDeleted(d) => write!(f, "KeyDeleted(key={})", d.key),
203+
Notification::KeyModified(m) => write!(
204+
f,
205+
"KeyModified(key={}, version_id={:?})",
206+
m.key, m.version_id
207+
),
208+
Notification::KeyRangeDeleted(r) => write!(
209+
f,
210+
"KeyRangeDeleted(key={}, last={:?})",
211+
r.key, r.key_range_last
212+
),
213+
Notification::Unknown() => write!(f, "Unknown"),
214+
}
215+
}
216+
}
217+
193218
impl From<(String, crate::oxia::Notification)> for Notification {
194219
fn from(tuple: (String, crate::oxia::Notification)) -> Self {
195220
let (key, notification) = tuple;

liboxia-native/src/shard_manager.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,18 +157,19 @@ impl ShardManager {
157157
pub fn get_shard(&self, key: &str) -> Option<i64> {
158158
let code = xxhash_rust::xxh32::xxh32(key.as_bytes(), 0);
159159
for entry in self.inner.current_assignments.iter() {
160-
match entry.shard_boundaries.unwrap() {
161-
ShardBoundaries::Int32HashRange(range) => {
162-
if range.min_hash_inclusive <= code && code <= range.max_hash_inclusive {
163-
return Some(entry.shard);
160+
if let Some(boundaries) = entry.shard_boundaries.as_ref() {
161+
match boundaries {
162+
ShardBoundaries::Int32HashRange(range) => {
163+
if range.min_hash_inclusive <= code && code <= range.max_hash_inclusive {
164+
return Some(entry.shard);
165+
}
164166
}
165167
}
166168
}
167169
}
168170
None
169171
}
170172

171-
// todo: consider iterator to avoid clone
172173
pub fn get_shards_leader(&self) -> HashMap<i64, Node> {
173174
let mut map = HashMap::with_capacity(self.inner.current_assignments.len());
174175
for item in self.inner.current_assignments.iter() {
@@ -185,12 +186,14 @@ impl ShardManager {
185186
pub fn get_shard_leader(&self, key: &str) -> Option<Node> {
186187
let code = xxhash_rust::xxh32::xxh32(key.as_bytes(), 0);
187188
for entry in self.inner.current_assignments.iter() {
188-
match entry.shard_boundaries.unwrap() {
189-
ShardBoundaries::Int32HashRange(range) => {
190-
if range.min_hash_inclusive <= code && code <= range.max_hash_inclusive {
191-
return Some(Node {
192-
service_address: ensure_protocol(entry.leader.clone()),
193-
});
189+
if let Some(boundaries) = entry.shard_boundaries.as_ref() {
190+
match boundaries {
191+
ShardBoundaries::Int32HashRange(range) => {
192+
if range.min_hash_inclusive <= code && code <= range.max_hash_inclusive {
193+
return Some(Node {
194+
service_address: ensure_protocol(entry.leader.clone()),
195+
});
196+
}
194197
}
195198
}
196199
}

0 commit comments

Comments
 (0)