[ENH] Rename FragmentSeqNo to FragmentIdentifier.

This doesn't change anything about the code (yet), but we're going to be
having a FragmentIdentifier::SeqNo and FragmentIdentifier::UUID.

Author: rescrv

rust/wal3/src/batch_manager.rs

@@ -3,7 +3,7 @@ use std::time::{Duration, Instant};
 
 use tracing::Span;
 
-use crate::{Error, FragmentSeqNo, LogPosition, ManifestManager, ThrottleOptions};
+use crate::{Error, FragmentIdentifier, LogPosition, ManifestManager, ThrottleOptions};
 
 /////////////////////////////////////////// ManagerState ///////////////////////////////////////////
 
@@ -39,7 +39,7 @@ impl ManagerState {
         options: &ThrottleOptions,
         manifest_manager: &ManifestManager,
         record_count: usize,
-    ) -> Result<Option<(FragmentSeqNo, LogPosition)>, Error> {
+    ) -> Result<Option<(FragmentIdentifier, LogPosition)>, Error> {
         if self.next_write > Instant::now() {
             return Ok(None);
         }
@@ -139,7 +139,7 @@ impl BatchManager {
         manifest_manager: &ManifestManager,
     ) -> Result<
         Option<(
-            FragmentSeqNo,
+            FragmentIdentifier,
             LogPosition,
             Vec<(
                 Vec<Vec<u8>>,
@@ -292,7 +292,7 @@ mod tests {
         batch_manager.push_work(vec![vec![4, 5, 6]], tx, tracing::Span::current());
         let (seq_no, log_position, work) =
             batch_manager.take_work(&manifest_manager).unwrap().unwrap();
-        assert_eq!(seq_no, FragmentSeqNo(1));
+        assert_eq!(seq_no, FragmentIdentifier(1));
         assert_eq!(log_position.offset(), 1);
         assert_eq!(2, work.len());
         // Check batch 1

rust/wal3/src/bin/wal3-construct-garbage-from-manifest.rs

@@ -1,6 +1,6 @@
 use std::str::FromStr;
 
-use wal3::{FragmentSeqNo, Garbage, LogPosition, Manifest};
+use wal3::{FragmentIdentifier, Garbage, LogPosition, Manifest};
 
 fn main() {
     let args = std::env::args().skip(1).collect::<Vec<_>>();
@@ -17,7 +17,7 @@ fn main() {
     eprintln!("offset: {offset:#?}");
     let garbage = Garbage::bug_patch_construct_garbage_from_manifest(
         &manifest,
-        FragmentSeqNo(seq_no),
+        FragmentIdentifier(seq_no),
         LogPosition::from_offset(offset),
     );
     println!("{}", serde_json::to_string(&garbage).unwrap());

rust/wal3/src/copy.rs

@@ -5,7 +5,7 @@ use setsum::Setsum;
 
 use crate::reader::LogReader;
 use crate::{
-    prefixed_fragment_path, Error, FragmentSeqNo, Limits, LogPosition, LogWriterOptions, Manifest,
+    prefixed_fragment_path, Error, FragmentIdentifier, Limits, LogPosition, LogWriterOptions, Manifest,
 };
 
 pub async fn copy(
@@ -56,7 +56,7 @@ pub async fn copy(
                 .iter()
                 .map(|f| f.seq_no)
                 .min()
-                .unwrap_or(FragmentSeqNo::BEGIN),
+                .unwrap_or(FragmentIdentifier::BEGIN),
         );
         let manifest = Manifest {
             setsum,

rust/wal3/src/gc.rs

@@ -13,7 +13,7 @@ use chroma_storage::{
 use crate::manifest::unprefixed_snapshot_path;
 use crate::writer::OnceLogWriter;
 use crate::{
-    deserialize_setsum, prefixed_fragment_path, serialize_setsum, Error, Fragment, FragmentSeqNo,
+    deserialize_setsum, prefixed_fragment_path, serialize_setsum, Error, Fragment, FragmentIdentifier,
     GarbageCollectionOptions, LogPosition, LogWriterOptions, Manifest, ScrubError, Snapshot,
     SnapshotCache, SnapshotPointer, ThrottleOptions,
 };
@@ -25,8 +25,8 @@ pub struct Garbage {
     pub snapshots_to_drop: Vec<SnapshotPointer>,
     pub snapshots_to_make: Vec<Snapshot>,
     pub snapshot_for_root: Option<SnapshotPointer>,
-    pub fragments_to_drop_start: FragmentSeqNo,
-    pub fragments_to_drop_limit: FragmentSeqNo,
+    pub fragments_to_drop_start: FragmentIdentifier,
+    pub fragments_to_drop_limit: FragmentIdentifier,
     #[serde(
         deserialize_with = "deserialize_setsum",
         serialize_with = "serialize_setsum"
@@ -41,8 +41,8 @@ impl Garbage {
             snapshots_to_drop: Vec::new(),
             snapshots_to_make: Vec::new(),
             snapshot_for_root: None,
-            fragments_to_drop_start: FragmentSeqNo(0),
-            fragments_to_drop_limit: FragmentSeqNo(0),
+            fragments_to_drop_start: FragmentIdentifier(0),
+            fragments_to_drop_limit: FragmentIdentifier(0),
             setsum_to_discard: Setsum::default(),
             first_to_keep: LogPosition::from_offset(1),
         }
@@ -89,8 +89,8 @@ impl Garbage {
             snapshots_to_drop: vec![],
             snapshots_to_make: vec![],
             snapshot_for_root: None,
-            fragments_to_drop_start: FragmentSeqNo(0),
-            fragments_to_drop_limit: FragmentSeqNo(0),
+            fragments_to_drop_start: FragmentIdentifier(0),
+            fragments_to_drop_limit: FragmentIdentifier(0),
             setsum_to_discard: Setsum::default(),
             first_to_keep,
         };
@@ -261,7 +261,7 @@ impl Garbage {
         paths.into_iter().chain(
             (self.fragments_to_drop_start.0..self.fragments_to_drop_limit.0)
                 .map(move |seq_no| (seq_no, Arc::clone(&prefix)))
-                .map(|(seq_no, prefix)| prefixed_fragment_path(&prefix, FragmentSeqNo(seq_no))),
+                .map(|(seq_no, prefix)| prefixed_fragment_path(&prefix, FragmentIdentifier(seq_no))),
         )
     }
 
@@ -502,7 +502,7 @@ impl Garbage {
     // snapshot, recursively, until you find the first fragment.  That's your seq_no.
     pub fn bug_patch_construct_garbage_from_manifest(
         manifest: &Manifest,
-        seq_no: FragmentSeqNo,
+        seq_no: FragmentIdentifier,
         offset: LogPosition,
     ) -> Garbage {
         let mut garbage = Garbage {
@@ -582,11 +582,11 @@ mod tests {
         let manifest: Manifest = serde_json::from_str(manifest_json).unwrap();
         let output = Garbage::bug_patch_construct_garbage_from_manifest(
             &manifest,
-            FragmentSeqNo(806913),
+            FragmentIdentifier(806913),
             LogPosition::from_offset(900883),
         );
-        assert_eq!(output.fragments_to_drop_start, FragmentSeqNo(806913));
-        assert_eq!(output.fragments_to_drop_limit, FragmentSeqNo(806913));
+        assert_eq!(output.fragments_to_drop_start, FragmentIdentifier(806913));
+        assert_eq!(output.fragments_to_drop_limit, FragmentIdentifier(806913));
         assert_eq!(
             output.setsum_to_discard.hexdigest(),
             "c921d21a0820be5d3b6f2d90942648f2853188bb0e3c6a22fe3dbd81c1e1c380"

rust/wal3/src/lib.rs

@@ -91,7 +91,7 @@ pub enum Error {
     #[error("garbage collection: {0}")]
     GarbageCollection(String),
     #[error("garbage collection precondition failed: manifest missing this: {0}")]
-    GarbageCollectionPrecondition(SnapshotPointerOrFragmentSeqNo),
+    GarbageCollectionPrecondition(SnapshotPointerOrFragmentIdentifier),
     #[error("scrub error: {0}")]
     ScrubError(#[from] Box<ScrubError>),
     #[error("parquet error: {0}")]
@@ -141,29 +141,29 @@ impl chroma_error::ChromaError for Error {
 ///////////////////////////////////// SnapshotPointerOrFragment ////////////////////////////////////
 
 #[derive(Clone, Debug)]
-pub enum SnapshotPointerOrFragmentSeqNo {
+pub enum SnapshotPointerOrFragmentIdentifier {
     SnapshotPointer(SnapshotPointer),
-    FragmentSeqNo(u64),
+    FragmentIdentifier(u64),
     Stringy(String),
 }
 
-impl From<SnapshotPointer> for SnapshotPointerOrFragmentSeqNo {
+impl From<SnapshotPointer> for SnapshotPointerOrFragmentIdentifier {
     fn from(inner: SnapshotPointer) -> Self {
         Self::SnapshotPointer(inner)
     }
 }
 
-impl From<u64> for SnapshotPointerOrFragmentSeqNo {
+impl From<u64> for SnapshotPointerOrFragmentIdentifier {
     fn from(inner: u64) -> Self {
-        Self::FragmentSeqNo(inner)
+        Self::FragmentIdentifier(inner)
     }
 }
 
-impl std::fmt::Display for SnapshotPointerOrFragmentSeqNo {
+impl std::fmt::Display for SnapshotPointerOrFragmentIdentifier {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         match self {
             Self::SnapshotPointer(ptr) => write!(f, "Snapshot({:?})", ptr.path_to_snapshot),
-            Self::FragmentSeqNo(seq) => write!(f, "Fragment({})", *seq),
+            Self::FragmentIdentifier(seq) => write!(f, "Fragment({})", *seq),
             Self::Stringy(s) => write!(f, "Stringy({s})"),
         }
     }
@@ -187,7 +187,7 @@ pub enum ScrubError {
     #[error("CorruptFragment: {seq_no} {what}")]
     CorruptFragment {
         manifest: String,
-        seq_no: FragmentSeqNo,
+        seq_no: FragmentIdentifier,
         what: String,
     },
     #[error("MismatchedPath: {reference:?} expected {:?} got {:?}", reference.path, empirical.path)]
@@ -500,24 +500,24 @@ pub struct GarbageCollectionOptions {
     pub throttle: ThrottleOptions,
 }
 
-/////////////////////////////////////////// FragmentSeqNo //////////////////////////////////////////
+/////////////////////////////////////////// FragmentIdentifier //////////////////////////////////////////
 
-/// A FragmentSeqNo is an identifier that corresponds to the the number of fragments that have been
-/// issued prior to the segment with this FragmentSeqNo.
+/// A FragmentIdentifier is an identifier that corresponds to the the number of fragments that have been
+/// issued prior to the segment with this FragmentIdentifier.
 #[derive(
     Clone, Copy, Debug, PartialEq, Eq, Ord, PartialOrd, Hash, serde::Deserialize, serde::Serialize,
 )]
-pub struct FragmentSeqNo(pub u64);
+pub struct FragmentIdentifier(pub u64);
 
-impl FragmentSeqNo {
-    const BEGIN: FragmentSeqNo = FragmentSeqNo(1);
+impl FragmentIdentifier {
+    const BEGIN: FragmentIdentifier = FragmentIdentifier(1);
 
-    /// Returns the successor of this FragmentSeqNo, or None if this FragmentSeqNo is the maximum
+    /// Returns the successor of this FragmentIdentifier, or None if this FragmentIdentifier is the maximum
     pub fn successor(&self) -> Option<Self> {
         if self.0 == u64::MAX {
             None
         } else {
-            Some(FragmentSeqNo(self.0 + 1))
+            Some(FragmentIdentifier(self.0 + 1))
         }
     }
 
@@ -527,37 +527,37 @@ impl FragmentSeqNo {
     }
 }
 
-impl std::fmt::Display for FragmentSeqNo {
+impl std::fmt::Display for FragmentIdentifier {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.0)
     }
 }
 
-impl std::ops::Add<FragmentSeqNo> for u64 {
-    type Output = FragmentSeqNo;
+impl std::ops::Add<FragmentIdentifier> for u64 {
+    type Output = FragmentIdentifier;
 
-    fn add(self, rhs: FragmentSeqNo) -> Self::Output {
-        FragmentSeqNo(self.wrapping_add(rhs.0))
+    fn add(self, rhs: FragmentIdentifier) -> Self::Output {
+        FragmentIdentifier(self.wrapping_add(rhs.0))
     }
 }
 
-impl std::ops::Add<u64> for FragmentSeqNo {
-    type Output = FragmentSeqNo;
+impl std::ops::Add<u64> for FragmentIdentifier {
+    type Output = FragmentIdentifier;
 
     fn add(self, rhs: u64) -> Self::Output {
-        FragmentSeqNo(self.0.wrapping_add(rhs))
+        FragmentIdentifier(self.0.wrapping_add(rhs))
     }
 }
 
-impl std::ops::Sub<FragmentSeqNo> for FragmentSeqNo {
+impl std::ops::Sub<FragmentIdentifier> for FragmentIdentifier {
     type Output = u64;
 
-    fn sub(self, rhs: FragmentSeqNo) -> Self::Output {
+    fn sub(self, rhs: FragmentIdentifier) -> Self::Output {
         self.0.wrapping_sub(rhs.0)
     }
 }
 
-impl std::ops::AddAssign<u64> for FragmentSeqNo {
+impl std::ops::AddAssign<u64> for FragmentIdentifier {
     fn add_assign(&mut self, rhs: u64) {
         self.0 = self.0.wrapping_add(rhs);
     }
@@ -569,7 +569,7 @@ impl std::ops::AddAssign<u64> for FragmentSeqNo {
 #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
 pub struct Fragment {
     pub path: String,
-    pub seq_no: FragmentSeqNo,
+    pub seq_no: FragmentIdentifier,
     pub start: LogPosition,
     pub limit: LogPosition,
     pub num_bytes: u64,
@@ -614,7 +614,7 @@ pub fn fragment_prefix() -> String {
     "log/".to_string()
 }
 
-pub fn prefixed_fragment_path(prefix: &str, fragment_seq_no: FragmentSeqNo) -> String {
+pub fn prefixed_fragment_path(prefix: &str, fragment_seq_no: FragmentIdentifier) -> String {
     format!(
         "{}/{}Bucket={:016x}/FragmentSeqNo={:016x}.parquet",
         prefix,
@@ -624,20 +624,20 @@ pub fn prefixed_fragment_path(prefix: &str, fragment_seq_no: FragmentSeqNo) -> S
     )
 }
 
-pub fn unprefixed_fragment_path(fragment_seq_no: FragmentSeqNo) -> String {
+pub fn unprefixed_fragment_path(fragment_seq_no: FragmentIdentifier) -> String {
     format!(
         "log/Bucket={:016x}/FragmentSeqNo={:016x}.parquet",
         fragment_seq_no.bucket(),
         fragment_seq_no.0,
     )
 }
 
-pub fn parse_fragment_path(path: &str) -> Option<FragmentSeqNo> {
-    // FragmentSeqNo is always in the basename.
+pub fn parse_fragment_path(path: &str) -> Option<FragmentIdentifier> {
+    // FragmentIdentifier is always in the basename.
     let (_, basename) = path.rsplit_once('/')?;
     let fsn_equals_number = basename.strip_suffix(".parquet")?;
     let number = fsn_equals_number.strip_prefix("FragmentSeqNo=")?;
-    u64::from_str_radix(number, 16).ok().map(FragmentSeqNo)
+    u64::from_str_radix(number, 16).ok().map(FragmentIdentifier)
 }
 
 /////////////////////////////////////////////// tests //////////////////////////////////////////////
@@ -650,7 +650,7 @@ mod tests {
     fn paths() {
         assert_eq!(
             "THIS_IS_THE_COLLECTION/log/Bucket=0000000000000000/FragmentSeqNo=0000000000000001.parquet",
-            prefixed_fragment_path("THIS_IS_THE_COLLECTION", FragmentSeqNo(1))
+            prefixed_fragment_path("THIS_IS_THE_COLLECTION", FragmentIdentifier(1))
         );
     }
 }