feat: detect not-found via error in bool, null and geo index open (#9265)

* feat: detect not-found via error in bool, null and geo index open

* fix: error on inconsistent storage in bool and null index open

* fix: hw counter write -> read

lib/segment/src/common/flags/read_only_roaring_flags.rs

@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
 use common::mmap::AdviceSetting;
 use common::stored_bitslice::StoredBitSlice;
 use common::types::PointOffsetType;
-use common::universal_io::{OpenOptions, Populate, TypedStorage, UniversalRead};
+use common::universal_io::{OkNotFound, OpenOptions, Populate, TypedStorage, UniversalRead};
 use roaring::RoaringBitmap;
 
 use super::dynamic_stored_flags::{DynamicFlagsStatus, FLAGS_FILE, status_file};
@@ -42,12 +42,16 @@ impl ReadOnlyRoaringFlags {
     /// past it), and the set positions from the flags file — shared with the
     /// writable path via [`StoredBitSlice::iter_ones`].
     ///
+    /// Returns [`Ok(None)`] when the flag directory doesn't exist (the status
+    /// file is absent), matching the read path's never-create contract.
+    ///
     /// [1]: super::roaring_flags::RoaringFlags::new
-    pub fn open<S: UniversalRead>(fs: &S::Fs, directory: &Path) -> OperationResult<Self> {
+    pub fn open<S: UniversalRead>(fs: &S::Fs, directory: &Path) -> OperationResult<Option<Self>> {
         // Logical length: read the status struct directly. `StoredStruct` is
-        // write-bound, so go through the read-only `TypedStorage`.
+        // write-bound, so go through the read-only `TypedStorage`. A missing
+        // status file means the index isn't present, so map not-found to `None`.
         let status_path = status_file(directory);
-        let status = TypedStorage::<S, DynamicFlagsStatus>::open(
+        let Some(status) = TypedStorage::<S, DynamicFlagsStatus>::open(
             fs,
             &status_path,
             OpenOptions {
@@ -57,7 +61,11 @@ impl ReadOnlyRoaringFlags {
                 advice: AdviceSetting::Global,
             },
             Default::default(),
-        )?;
+        )
+        .ok_not_found()?
+        else {
+            return Ok(None);
+        };
         let len = status.read_whole()?[0].len();
 
         // Set positions: open the flags file and build the bitmap. The
@@ -81,11 +89,11 @@ impl ReadOnlyRoaringFlags {
         )
         .expect("iter_ones iterates in sorted order");
 
-        Ok(Self {
+        Ok(Some(Self {
             bitmap,
             len,
             directory: directory.to_path_buf(),
-        })
+        }))
     }
 }
 

lib/segment/src/index/field_index/bool_index/read_only_bool_index/lifecycle.rs

@@ -6,7 +6,7 @@ use super::super::mutable_bool_index::{FALSES_DIRNAME, TRUES_DIRNAME};
 use super::{ReadOnlyBoolIndex, ReadOnlyStorage};
 use crate::common::flags::read_only_roaring_flags::ReadOnlyRoaringFlags;
 use crate::common::flags::roaring_flags::RoaringFlagsRead;
-use crate::common::operation_error::OperationResult;
+use crate::common::operation_error::{OperationError, OperationResult};
 
 impl ReadOnlyBoolIndex {
     /// Open a read-only bool index at `path`, threading every file open through
@@ -19,22 +19,43 @@ impl ReadOnlyBoolIndex {
     /// (`|trues ∪ falses|`) is derived from the two bitmaps, so `open` takes
     /// only `fs` and the directory.
     ///
+    /// Returns [`Ok(None)`] only when both flag directories are absent. If
+    /// exactly one of `trues` / `falses` exists, the on-disk layout is
+    /// partial/corrupt: it surfaces as an error rather than a silently-missing
+    /// index that would drop the persisted postings of the present half.
+    ///
     /// [1]: super::super::mutable_bool_index::MutableBoolIndex::open
-    pub fn open<S: UniversalRead>(fs: &S::Fs, path: &Path) -> OperationResult<Self> {
+    pub fn open<S: UniversalRead>(fs: &S::Fs, path: &Path) -> OperationResult<Option<Self>> {
+        // Open both directories first so a partial layout can be distinguished
+        // from a genuinely absent index, regardless of which half is missing.
         let trues_flags = ReadOnlyRoaringFlags::open::<S>(fs, &path.join(TRUES_DIRNAME))?;
         let falses_flags = ReadOnlyRoaringFlags::open::<S>(fs, &path.join(FALSES_DIRNAME))?;
 
-        let indexed_count = trues_flags
-            .get_bitmap()
-            .union_len(falses_flags.get_bitmap()) as usize;
+        match (trues_flags, falses_flags) {
+            // Neither directory exists: the index isn't present on disk.
+            (None, None) => Ok(None),
+            (Some(trues_flags), Some(falses_flags)) => {
+                let indexed_count = trues_flags
+                    .get_bitmap()
+                    .union_len(falses_flags.get_bitmap())
+                    as usize;
 
-        Ok(Self {
-            _base_dir: path.to_path_buf(),
-            storage: ReadOnlyStorage {
-                trues_flags,
-                falses_flags,
-            },
-            indexed_count,
-        })
+                Ok(Some(Self {
+                    _base_dir: path.to_path_buf(),
+                    storage: ReadOnlyStorage {
+                        trues_flags,
+                        falses_flags,
+                    },
+                    indexed_count,
+                }))
+            }
+            // Exactly one directory exists: partial/corrupt storage.
+            (trues, falses) => Err(OperationError::service_error(format!(
+                "inconsistent bool index at {path:?}: exactly one flag directory exists \
+                 ({TRUES_DIRNAME}: {}, {FALSES_DIRNAME}: {})",
+                trues.is_some(),
+                falses.is_some(),
+            ))),
+        }
     }
 }

lib/segment/src/index/field_index/bool_index/read_only_bool_index/mod.rs

@@ -68,7 +68,7 @@ mod tests {
     use serde_json::json;
     use tempfile::TempDir;
 
-    use super::super::mutable_bool_index::MutableBoolIndex;
+    use super::super::mutable_bool_index::{FALSES_DIRNAME, MutableBoolIndex, TRUES_DIRNAME};
     use super::ReadOnlyBoolIndex;
     use crate::index::field_index::{
         FieldIndexBuilderTrait, PayloadFieldIndex, PayloadFieldIndexRead,
@@ -124,7 +124,9 @@ mod tests {
         type RoFs = <ReadOnly<MmapFile> as UniversalRead>::Fs;
         let fs = RoFs::from_context(Default::default()).unwrap();
 
-        let index = ReadOnlyBoolIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path()).unwrap();
+        let index = ReadOnlyBoolIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path())
+            .unwrap()
+            .unwrap();
 
         let hw_acc = HwMeasurementAcc::new();
         let hw_counter = hw_acc.get_counter_cell();
@@ -148,4 +150,34 @@ mod tests {
         // `indexed_count = |trues ∪ falses|`, derived from the two bitmaps on open.
         assert_eq!(index.count_indexed_points(), 9);
     }
+
+    /// A partial on-disk layout — exactly one of the `trues` / `falses` flag
+    /// directories present — is corrupt storage: `open` surfaces an error in
+    /// either direction, rather than silently reporting a missing index.
+    #[test]
+    fn open_inconsistent_storage_errors() {
+        type RoFs = <ReadOnly<MmapFile> as UniversalRead>::Fs;
+        let fs = RoFs::from_context(Default::default()).unwrap();
+
+        let build = || {
+            let dir = TempDir::with_prefix("read_only_bool_inconsistent").unwrap();
+            let hw_counter = HardwareCounterCell::new();
+            let mut builder = MutableBoolIndex::builder(dir.path()).unwrap();
+            let value = json!(true);
+            builder.add_point(0, &[&value], &hw_counter).unwrap();
+            let index = builder.finalize().unwrap();
+            index.flusher()().unwrap();
+            dir
+        };
+
+        // `trues` present, `falses` removed.
+        let dir = build();
+        fs_err::remove_dir_all(dir.path().join(FALSES_DIRNAME)).unwrap();
+        assert!(ReadOnlyBoolIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path()).is_err());
+
+        // `falses` present, `trues` removed.
+        let dir = build();
+        fs_err::remove_dir_all(dir.path().join(TRUES_DIRNAME)).unwrap();
+        assert!(ReadOnlyBoolIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path()).is_err());
+    }
 }

lib/segment/src/index/field_index/geo_index/mutable_geo_index/read_only/lifecycle.rs

@@ -1,7 +1,7 @@
 use std::path::PathBuf;
 
 use common::counter::hardware_counter::HardwareCounterCell;
-use common::universal_io::UniversalRead;
+use common::universal_io::{OkNotFound, UniversalRead};
 use gridstore::GridstoreReader;
 
 use super::super::inner::InMemoryGeoMapIndex;
@@ -20,13 +20,18 @@ impl<S: UniversalRead> ReadOnlyAppendableGeoMapIndex<S> {
     /// performs over a writable `Gridstore`. No write path; the reader is
     /// retained for `files` / `clear_cache`.
     ///
+    /// Returns [`Ok(None)`] when the on-disk directory doesn't exist, matching
+    /// the `create_if_missing == false` branch of the writable counterpart —
+    /// the read path never creates.
+    ///
     /// [1]: super::super::MutableGeoMapIndex::open_gridstore
-    pub fn open(fs: &S::Fs, path: PathBuf) -> OperationResult<Self> {
-        let storage = GridstoreReader::<Vec<RawGeoPoint>, S>::open(fs, path).map_err(|err| {
-            OperationError::service_error(format!(
-                "failed to open read-only appendable geo index on gridstore: {err}"
-            ))
-        })?;
+    pub fn open(fs: &S::Fs, path: PathBuf) -> OperationResult<Option<Self>> {
+        let Some(storage) =
+            GridstoreReader::<Vec<RawGeoPoint>, S>::open(fs, path).ok_not_found()?
+        else {
+            // Files don't exist, cannot load
+            return Ok(None);
+        };
 
         let mut in_memory_index = InMemoryGeoMapIndex::new();
         let hw_counter = HardwareCounterCell::disposable();
@@ -39,7 +44,7 @@ impl<S: UniversalRead> ReadOnlyAppendableGeoMapIndex<S> {
                 },
                 // Same counter the writable `open_gridstore` load uses; this is
                 // a disposable counter, so the exact metric is unobservable.
-                hw_counter.ref_payload_index_io_write_counter(),
+                hw_counter.ref_payload_index_io_read_counter(),
             )
             .map_err(|err| {
                 OperationError::service_error(format!(
@@ -47,9 +52,9 @@ impl<S: UniversalRead> ReadOnlyAppendableGeoMapIndex<S> {
                 ))
             })?;
 
-        Ok(Self {
+        Ok(Some(Self {
             in_memory_index,
             storage,
-        })
+        }))
     }
 }

lib/segment/src/index/field_index/geo_index/mutable_geo_index/read_only/mod.rs

@@ -76,7 +76,9 @@ mod tests {
         type RoFs = <ReadOnly<MmapFile> as UniversalRead>::Fs;
         let fs = RoFs::from_context(Default::default()).unwrap();
         let index: ReadOnlyAppendableGeoMapIndex<ReadOnly<MmapFile>> =
-            ReadOnlyAppendableGeoMapIndex::open(&fs, dir.path().to_path_buf()).unwrap();
+            ReadOnlyAppendableGeoMapIndex::open(&fs, dir.path().to_path_buf())
+                .unwrap()
+                .unwrap();
 
         // Counts reconstructed from the Gridstore on open.
         assert_eq!(index.points_count(), 3);

lib/segment/src/index/field_index/geo_index/read_only/lifecycle.rs

@@ -18,10 +18,8 @@ impl<S: UniversalRead> ReadOnlyGeoMapIndex<S> {
     /// uniformly. No `create_if_missing`: the read path never creates.
     ///
     /// [1]: super::super::GeoMapIndex::new_gridstore
-    pub fn open_gridstore(fs: &S::Fs, dir: PathBuf) -> OperationResult<Self> {
-        Ok(Self::Appendable(ReadOnlyAppendableGeoMapIndex::open(
-            fs, dir,
-        )?))
+    pub fn open_gridstore(fs: &S::Fs, dir: PathBuf) -> OperationResult<Option<Self>> {
+        Ok(ReadOnlyAppendableGeoMapIndex::open(fs, dir)?.map(Self::Appendable))
     }
 
     /// Read-only mirror of [`GeoMapIndex::new_mmap`][1]: open the immutable

lib/segment/src/index/field_index/geo_index/read_only/mod.rs

@@ -126,7 +126,9 @@ mod tests {
         type RoFs = <ReadOnly<MmapFile> as UniversalRead>::Fs;
         let fs = RoFs::from_context(Default::default()).unwrap();
         let index: ReadOnlyGeoMapIndex<ReadOnly<MmapFile>> =
-            ReadOnlyGeoMapIndex::open_gridstore(&fs, dir.path().to_path_buf()).unwrap();
+            ReadOnlyGeoMapIndex::open_gridstore(&fs, dir.path().to_path_buf())
+                .unwrap()
+                .unwrap();
 
         // Dispatcher wraps the leaf into the right variant.
         assert!(matches!(index, ReadOnlyGeoMapIndex::Appendable(_)));

lib/segment/src/index/field_index/null_index/read_only_null_index/lifecycle.rs

@@ -5,7 +5,7 @@ use common::universal_io::UniversalRead;
 use super::super::mutable_null_index::{HAS_VALUES_DIRNAME, IS_NULL_DIRNAME};
 use super::{ReadOnlyNullIndex, ReadOnlyStorage};
 use crate::common::flags::read_only_roaring_flags::ReadOnlyRoaringFlags;
-use crate::common::operation_error::OperationResult;
+use crate::common::operation_error::{OperationError, OperationResult};
 
 impl ReadOnlyNullIndex {
     /// Open a read-only null index at `path`, threading every file open through
@@ -18,22 +18,40 @@ impl ReadOnlyNullIndex {
     /// writable [`MutableNullIndex::open`][1] receives; it is not derivable from
     /// the index files alone.
     ///
+    /// Returns [`Ok(None)`] only when both flag directories are absent. If
+    /// exactly one of `has_values` / `is_null` exists, the on-disk layout is
+    /// partial/corrupt: it surfaces as an error rather than a silently-missing
+    /// index that would drop the persisted postings of the present half.
+    ///
     /// [1]: super::super::mutable_null_index::MutableNullIndex::open
     pub fn open<S: UniversalRead>(
         fs: &S::Fs,
         path: &Path,
         total_point_count: usize,
-    ) -> OperationResult<Self> {
+    ) -> OperationResult<Option<Self>> {
+        // Open both directories first so a partial layout can be distinguished
+        // from a genuinely absent index, regardless of which half is missing.
         let has_values_flags = ReadOnlyRoaringFlags::open::<S>(fs, &path.join(HAS_VALUES_DIRNAME))?;
         let is_null_flags = ReadOnlyRoaringFlags::open::<S>(fs, &path.join(IS_NULL_DIRNAME))?;
 
-        Ok(Self {
-            _base_dir: path.to_path_buf(),
-            storage: ReadOnlyStorage {
-                has_values_flags,
-                is_null_flags,
-            },
-            total_point_count,
-        })
+        match (has_values_flags, is_null_flags) {
+            // Neither directory exists: the index isn't present on disk.
+            (None, None) => Ok(None),
+            (Some(has_values_flags), Some(is_null_flags)) => Ok(Some(Self {
+                _base_dir: path.to_path_buf(),
+                storage: ReadOnlyStorage {
+                    has_values_flags,
+                    is_null_flags,
+                },
+                total_point_count,
+            })),
+            // Exactly one directory exists: partial/corrupt storage.
+            (has_values, is_null) => Err(OperationError::service_error(format!(
+                "inconsistent null index at {path:?}: exactly one flag directory exists \
+                 ({HAS_VALUES_DIRNAME}: {}, {IS_NULL_DIRNAME}: {})",
+                has_values.is_some(),
+                is_null.is_some(),
+            ))),
+        }
     }
 }

lib/segment/src/index/field_index/null_index/read_only_null_index/mod.rs

@@ -63,7 +63,7 @@ mod tests {
     use serde_json::{Value, json};
     use tempfile::TempDir;
 
-    use super::super::mutable_null_index::MutableNullIndex;
+    use super::super::mutable_null_index::{HAS_VALUES_DIRNAME, IS_NULL_DIRNAME, MutableNullIndex};
     use super::super::read_ops::NullIndexRead;
     use super::ReadOnlyNullIndex;
     use crate::index::field_index::{FieldIndexBuilderTrait, PayloadFieldIndexRead};
@@ -95,7 +95,9 @@ mod tests {
         type RoFs = <ReadOnly<MmapFile> as UniversalRead>::Fs;
         let fs = RoFs::from_context(Default::default()).unwrap();
 
-        let index = ReadOnlyNullIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path(), total).unwrap();
+        let index = ReadOnlyNullIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path(), total)
+            .unwrap()
+            .unwrap();
 
         let key = JsonPath::new("test");
         let is_null = FieldCondition::new_is_null(key.clone(), true);
@@ -161,4 +163,33 @@ mod tests {
         assert!(index.values_is_null(1));
         assert!(!index.values_is_null(3));
     }
+
+    /// A partial on-disk layout — exactly one of the `has_values` / `is_null`
+    /// flag directories present — is corrupt storage: `open` surfaces an error
+    /// in either direction, rather than silently reporting a missing index.
+    #[test]
+    fn open_inconsistent_storage_errors() {
+        type RoFs = <ReadOnly<MmapFile> as UniversalRead>::Fs;
+        let fs = RoFs::from_context(Default::default()).unwrap();
+
+        let build = || {
+            let dir = TempDir::with_prefix("read_only_null_inconsistent").unwrap();
+            let hw_counter = HardwareCounterCell::new();
+            let mut builder = MutableNullIndex::builder(dir.path(), 0).unwrap();
+            let value = json!(true);
+            builder.add_point(0, &[&value], &hw_counter).unwrap();
+            builder.finalize().unwrap();
+            dir
+        };
+
+        // `has_values` present, `is_null` removed.
+        let dir = build();
+        fs_err::remove_dir_all(dir.path().join(IS_NULL_DIRNAME)).unwrap();
+        assert!(ReadOnlyNullIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path(), 1).is_err());
+
+        // `is_null` present, `has_values` removed.
+        let dir = build();
+        fs_err::remove_dir_all(dir.path().join(HAS_VALUES_DIRNAME)).unwrap();
+        assert!(ReadOnlyNullIndex::open::<ReadOnly<MmapFile>>(&fs, dir.path(), 1).is_err());
+    }
 }