Repository: qdrant/qdrant
File: lib/segment/src/id_tracker/id_tracker_base.rs (Rust)

Code before:
            (0..total)
                .map(move |_| rng.random_range(0..total))
                .filter(move |&x| {
                    // Check for deleted vector first, as that is more likely
                    !deleted_vector_bitslice
                        .and_then(|d| d.get_bit(x as usize))
                        .unwrap_or(false)
                    // Also check point deletion for integrity
                    && !self.is_deleted_point(x)
                }),
        )
    }

    /// Iterate over all stored internal versions, even if they were deleted
    /// Required for cleanup on segment open
    fn iter_internal_versions(
        &self,
    ) -> Box<dyn Iterator<Item = (PointOffsetType, SeqNumberType)> + '_>;

    /// Finds inconsistencies between id mapping and versions storage.
    /// It might happen that point doesn't have version due to un-flushed WAL.
    /// This method makes those points usable again.
    ///
    /// Returns a list of internal ids, that have non-zero versions, but are missing in the id mapping.
    /// Those points should be removed from all other parts of the segment.
    fn cleanup_versions(&mut self) -> OperationResult<Vec<PointOffsetType>> {
        // Points with mapping, but no version.
        // Can happen if insertion didn't complete.
        // We need to remove mapping and assume the point is going to be re-inserted by WAL.
        let mut to_remove = Vec::new();

        // Points with version, but no mapping.
        // Can happen if point was deleted, but version (and likely the storage) wasn't cleaned up.
        // We return those points to the caller to clean up the storage.
        let mut to_return = Vec::new();

        for (internal_id, version) in self.iter_internal_versions() {
            if version != DELETED_POINT_VERSION && self.external_id(internal_id).is_none() {
                to_return.push(internal_id);
            }
        }

        for internal_id in self.iter_internal() {
            if self.internal_version(internal_id).is_none() {
                if let Some(external_id) = self.external_id(internal_id) {
                    to_remove.push(external_id);
                    to_return.push(internal_id);
                } else {
                    debug_assert!(false, "internal id {internal_id} has no external id");
                }
            }

Review comment (suggestion):
Bit of a nitpick, but this function name suggests me it cleans stuff up for us - it doesn't.

Maybe it's reasonable to rename it to something like `orphan_versions()`

Code after:
        Box::new(
            (0..total)
                .map(move |_| rng.random_range(0..total))
                .filter(move |&x| {
                    // Check for deleted vector first, as that is more likely
                    !deleted_vector_bitslice
                        .and_then(|d| d.get_bit(x as usize))
                        .unwrap_or(false)
                    // Also check point deletion for integrity
                    && !self.is_deleted_point(x)
                }),
        )
    }

    /// Iterate over all stored internal versions, even if they were deleted
    /// Required for cleanup on segment open
    fn iter_internal_versions(
        &self,
    ) -> Box<dyn Iterator<Item = (PointOffsetType, SeqNumberType)> + '_>;

    /// Finds inconsistencies between id mapping and versions storage.
    /// It might happen that point doesn't have version due to un-flushed WAL.
    /// This method makes those points usable again.
    ///
    /// Returns a list of internal ids, that have non-zero versions, but are missing in the id mapping.
    /// Those points should be removed from all other parts of the segment.
    fn fix_inconsistencies(&mut self) -> OperationResult<Vec<PointOffsetType>> {
        // Points with mapping, but no version.
        // Can happen if insertion didn't complete.
        // We need to remove mapping and assume the point is going to be re-inserted by WAL.
        let mut to_remove = Vec::new();

        // Points with version, but no mapping.
        // Can happen if point was deleted, but version (and likely the storage) wasn't cleaned up.
        // We return those points to the caller to clean up the storage.
        let mut to_return = Vec::new();

        for (internal_id, version) in self.iter_internal_versions() {
            if version != DELETED_POINT_VERSION && self.external_id(internal_id).is_none() {
                to_return.push(internal_id);
            }
        }

        for internal_id in self.iter_internal() {
            if self.internal_version(internal_id).is_none() {
                if let Some(external_id) = self.external_id(internal_id) {
                    to_remove.push(external_id);
                    to_return.push(internal_id);
                } else {
                    debug_assert!(false, "internal id {internal_id} has no external id");
                }