Skip to content

Commit 56a6eba

Browse files
author
Mark Hildebrand
committed
Force bftree insert errors to be handled.
1 parent b5ebac2 commit 56a6eba

7 files changed

Lines changed: 76 additions & 21 deletions

File tree

diskann-bftree/.clippy.toml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
allow-unwrap-in-tests = true
2+
allow-expect-in-tests = true
3+
allow-panic-in-tests = true
4+
5+
disallowed-methods = [
6+
{ path = "bf_tree::BfTree::insert", reason = "This method is fallible but returns a `bool`. Use `crate::bftree_insert` instead" }
7+
]

diskann-bftree/Cargo.toml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ tokio = { workspace = true, features = ["full"] }
3737
default = []
3838
experimental_diversity_search = ["diskann/experimental_diversity_search"]
3939

40-
[lints]
41-
workspace = true
40+
[lints.clippy]
41+
undocumented_unsafe_blocks = "warn"
42+
unwrap_used = "warn"
43+
expect_used = "warn"
44+
panic = "warn"
45+
disallowed_methods = "warn"
46+
4247

diskann-bftree/src/lib.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,3 +160,38 @@ impl Default for TestCallCount {
160160
Self::new()
161161
}
162162
}
163+
164+
/// `bf_tree::BfTree::insert` can fail, but uses a `LeafInsertResult` that is not `#[must_use]`.
165+
///
166+
/// This makes it too easy to drop errors.
167+
///
168+
/// We use a `clippy` lint to explicitly disallow `bf_tree::BfTree::insert` and instead
169+
/// funnel calls through this method instead to get a proper error.
170+
#[expect(
171+
clippy::disallowed_methods,
172+
reason = "this is the allowed way to call this method"
173+
)]
174+
fn bftree_insert(tree: &bf_tree::BfTree, key: &[u8], value: &[u8]) -> Result<(), InsertError> {
175+
match tree.insert(key, value) {
176+
bf_tree::LeafInsertResult::Success => Ok(()),
177+
bf_tree::LeafInsertResult::InvalidKV(s) => Err(InsertError(s)),
178+
}
179+
}
180+
181+
#[derive(Debug)]
182+
pub struct InsertError(String);
183+
184+
impl std::fmt::Display for InsertError {
185+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
186+
write!(f, "insert into a `bftree` failed: {}", self.0)
187+
}
188+
}
189+
190+
impl std::error::Error for InsertError {}
191+
192+
impl From<InsertError> for ANNError {
193+
#[track_caller]
194+
fn from(error: InsertError) -> Self {
195+
ANNError::new(diskann::ANNErrorKind::IndexError, error)
196+
}
197+
}

diskann-bftree/src/neighbors.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use diskann::{
1717
};
1818

1919
use super::ConfigError;
20-
use crate::TestCallCount;
20+
use crate::{bftree_insert, TestCallCount};
2121

2222
pub struct NeighborProvider<I: VectorId> {
2323
adjacency_list_index: BfTree,
@@ -142,10 +142,10 @@ impl<I: VectorId> NeighborProvider<I> {
142142
///
143143
/// Each key is a `VectorId`, written in bytes. The value is a
144144
/// neighbor list of length exactly `self.dim`. Specifically,
145-
/// an array of exactly `n` neighbors is written as :
145+
/// an array of exactly `n` neighbors is written as :
146146
/// ```text
147147
/// | I0 | ... | In | padding | ... | [n; 0] |
148-
///
148+
///
149149
/// -----------------------------------------------
150150
/// |
151151
/// self.dim
@@ -177,7 +177,7 @@ impl<I: VectorId> NeighborProvider<I> {
177177
let key = bytemuck::bytes_of(&vector_id);
178178
let value = cast_slice::<I, u8>(&buf[..self.dim]);
179179

180-
self.adjacency_list_index.insert(key, value);
180+
bftree_insert(&self.adjacency_list_index, key, value)?;
181181

182182
Ok(())
183183
}
@@ -189,7 +189,7 @@ impl<I: VectorId> NeighborProvider<I> {
189189
/// bytes of it into the bf-tree.
190190
///
191191
/// # Errors
192-
///
192+
///
193193
/// - Neighbor length is larger than `self.max_degree()`
194194
/// - Buffer length (in `I` cells) is smaller than `max_degree() + 1`
195195
pub fn set_neighbors(&self, vector_id: I, neighbors: &[I], buf: &mut [I]) -> ANNResult<()> {

diskann-bftree/src/provider.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,10 +862,17 @@ where
862862
}
863863

864864
fn get_distance(&mut self, id: u32) -> Result<f32, AccessError> {
865-
self.provider
865+
match self
866+
.provider
866867
.quant_vectors
867868
.get_vector_into(id.into_usize(), &mut self.element)
868-
.map(|_: ()| self.computer.evaluate_similarity(&self.element))
869+
{
870+
Ok(()) => self
871+
.computer
872+
.evaluate(&self.element)
873+
.map_err(RankedError::Error),
874+
Err(err) => Err(err),
875+
}
869876
}
870877
}
871878

diskann-bftree/src/quant.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,16 @@ use diskann_quantization::{
1717
use diskann_vector::PreprocessedDistanceFunction;
1818

1919
use super::ConfigError;
20-
use crate::TestCallCount;
20+
use crate::{bftree_insert, TestCallCount};
2121

2222
pub struct QuantQueryComputer(QueryComputer<GlobalAllocator>);
2323

24-
impl PreprocessedDistanceFunction<&[u8], f32> for QuantQueryComputer {
25-
fn evaluate_similarity(&self, x: &[u8]) -> f32 {
26-
self.0
27-
.evaluate_similarity(Opaque::new(x))
28-
.expect("spherical query distance failed")
24+
impl QuantQueryComputer {
25+
pub(crate) fn evaluate(&self, x: &[u8]) -> ANNResult<f32> {
26+
match self.0.evaluate_similarity(Opaque::new(x)) {
27+
Ok(distance) => Ok(distance),
28+
Err(err) => Err(ANNError::new(diskann::ANNErrorKind::IndexError, err)),
29+
}
2930
}
3031
}
3132

@@ -188,7 +189,7 @@ impl QuantVectorProvider {
188189
)
189190
.map_err(|e| ANNError::log_sq_error(e))?;
190191

191-
self.quant_vector_index.insert(key, quant_vector);
192+
bftree_insert(&self.quant_vector_index, key, quant_vector)?;
192193

193194
Ok(())
194195
}
@@ -209,7 +210,7 @@ impl QuantVectorProvider {
209210
// Update pq vector with id = i to v
210211
let key = bytemuck::bytes_of(&i);
211212

212-
self.quant_vector_index.insert(key, v);
213+
bftree_insert(&self.quant_vector_index, key, v)?;
213214

214215
Ok(())
215216
}
@@ -264,7 +265,7 @@ mod tests {
264265

265266
use diskann::ANNErrorKind;
266267
use diskann_quantization::spherical::iface::Opaque;
267-
use diskann_vector::{DistanceFunction, PreprocessedDistanceFunction};
268+
use diskann_vector::DistanceFunction;
268269
use tokio::task::JoinSet;
269270

270271
use super::*;
@@ -331,7 +332,7 @@ mod tests {
331332

332333
// Query Computer — verify it returns finite distances.
333334
let c = provider.query_computer(&[-0.5f32, -0.5]).unwrap();
334-
let dist = c.evaluate_similarity(&provider.get_vector_sync(3).unwrap());
335+
let dist = c.evaluate(&provider.get_vector_sync(3).unwrap()).unwrap();
335336
assert!(dist.is_finite(), "query distance should be finite");
336337

337338
// Distance Computer — verify distances between compressed vectors are finite

diskann-bftree/src/vectors.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use diskann::{error::RankedError, utils::VectorRepr, ANNError, ANNErrorKind, ANN
1414
use thiserror::Error;
1515

1616
use super::ConfigError;
17-
use crate::TestCallCount;
17+
use crate::{bftree_insert, TestCallCount};
1818

1919
pub struct VectorProvider<T: VectorRepr> {
2020
dim: usize,
@@ -131,7 +131,7 @@ impl<T: VectorRepr> VectorProvider<T> {
131131
let key = bytemuck::bytes_of(&i);
132132
let value = cast_slice::<T, u8>(v);
133133

134-
self.vector_index.insert(key, value);
134+
bftree_insert(&self.vector_index, key, value)?;
135135

136136
Ok(())
137137
}

0 commit comments

Comments
 (0)