Merge branch 'main' into empty-commits

* main:
  Error fast when can_write is false and calling gc + expiration (#1014)
  Document cold buckets and GCS 429 problem (#1009)
  Add initial commit id as constant (#1008)
  Fix typo (#1010)

Author: dcherian

docs/docs/icechunk-python/performance.md

@@ -9,6 +9,36 @@
 
 Icechunk is designed to be cloud native, making it able to take advantage of the horizontal scaling of cloud providers. To learn more, check out [this blog post](https://earthmover.io/blog/exploring-icechunk-scalability) which explores just how well Icechunk can perform when matched with AWS S3.
 
+## Cold buckets and repos
+
+Modern object stores usually reshard their buckets on-the-fly, based on perceived load. The
+strategies they use are not published and very hard to discover. The details are not super important
+anyway, the important take away is that on new buckets and even on new repositories, the scalability
+of the object store may not be great from the start. You are expected to slowly ramp up load, as you
+write data to the repository.
+
+Once you have applied consistently high write/read load to a repository for a few minutes, the object
+store will usually reshard your bucket allowing for more load. While this resharding happens, different
+object stores can respond in different ways. For example, S3 returns 5xx errors with a "SlowDown"
+indication. GCS returns 429 responses.
+
+Icechunk helps this process by retrying failed requests with an exponential backoff. In our
+experience, the default configuration is enough to ingest into a fresh bucket using around 100 machines.
+But if this is not the case for you, you can tune the retry configuration using [StorageRetriesSettings](https://icechunk.io/en/latest/icechunk-python/reference/#icechunk.StorageRetriesSettings).
+
+To learn more about how Icechunk manages object store prefixes, read our
+[blog post](https://earthmover.io/blog/exploring-icechunk-scalability)
+on Icechunk scalability.
+
+!!! warning
+
+    Currently, Icechunk implementation of retry logic during resharding is not
+    [working properly](https://github.com/earth-mover/icechunk/issues/954) on GCS.
+    We have a [pull request open](https://github.com/apache/arrow-rs-object-store/pull/410) to
+    one of Icechunk's dependencies that will solve this.
+    In the meantime, if you get 429 errors from your Google bucket, please lower concurrency and try
+    again. Increase concurrency slowly until errors disappear.
+
 ## Preloading manifests
 
 Coming Soon.
@@ -65,8 +95,8 @@ Options for specifying the arrays whose manifest you want to split are:
 3. [`ManifestSplitCondition.and_conditions`](./reference.md#icechunk.ManifestSplitCondition.and_conditions) to combine (1), (2), and (4) together; and
 4. [`ManifestSplitCondition.or_conditions`](./reference.md#icechunk.ManifestSplitCondition.or_conditions) to combine (1), (2), and (3) together.
 
-
 `And` and `Or` may be used to combine multiple path and/or name matches. For example,
+
 ```python exec="on" session="perf" source="material-block"
 array_condition = ManifestSplitCondition.or_conditions(
     [
@@ -85,8 +115,8 @@ Options for specifying how to split along a specific axis or dimension are:
 2. [`ManifestSplitDimCondition.DimensionName`](./reference.md#icechunk.ManifestSplitDimCondition.DimensionName) takes a regular expression used to match the dimension names of the array;
 3. [`ManifestSplitDimCondition.Any`](./reference.md#icechunk.ManifestSplitDimCondition.Any) matches any _remaining_ dimension name or axis.
 
-
 For example, for an array with dimensions `time, latitude, longitude`, the following config
+
 ```python exec="on" session="perf" source="material-block"
 from icechunk import ManifestSplitDimCondition
 
@@ -96,8 +126,8 @@ from icechunk import ManifestSplitDimCondition
     ManifestSplitDimCondition.Any(): 1,
 }
 ```
-will result in splitting manifests so that each manifest contains (3 longitude chunks x 2 latitude chunks x 1 time chunk) = 6 chunks per manifest file.
 
+will result in splitting manifests so that each manifest contains (3 longitude chunks x 2 latitude chunks x 1 time chunk) = 6 chunks per manifest file.
 
 !!! note
 

docs/docs/icechunk-python/virtual.md

@@ -120,7 +120,7 @@ Now we can read the dataset from the store using xarray to confirm everything we
 
 ```python
 ds = xr.open_zarr(
-    store,
+    session.store,
     zarr_version=3,
     consolidated=False,
     chunks={},

icechunk-python/tests/test_timetravel.py

@@ -124,6 +124,7 @@ def test_timetravel() -> None:
         "commit 1",
         "Repository initialized",
     ]
+    assert parents[-1].id == "1CECHNKREP0F1RSTCMT0"
     assert [len(snap.manifests) for snap in parents] == [1, 1, 1, 0]
     assert sorted(parents, key=lambda p: p.written_at) == list(reversed(parents))
     assert len(set([snap.id for snap in parents])) == 4

icechunk/src/format/mod.rs

@@ -98,7 +98,7 @@ impl<const SIZE: usize, T: FileTypeTag> ObjectId<SIZE, T> {
         Self(buf, PhantomData)
     }
 
-    pub fn new(buf: [u8; SIZE]) -> Self {
+    pub const fn new(buf: [u8; SIZE]) -> Self {
         Self(buf, PhantomData)
     }
 

icechunk/src/format/snapshot.rs

@@ -340,6 +340,10 @@ static ROOT_OPTIONS: VerifierOptions = VerifierOptions {
 
 impl Snapshot {
     pub const INITIAL_COMMIT_MESSAGE: &'static str = "Repository initialized";
+    pub const INITIAL_SNAPSHOT_ID: SnapshotId = SnapshotId::new([
+        0x0b, 0x1c, 0xc8, 0xd6, 0x78, 0x75, 0x80, 0xf0, 0xe3, 0x3a, 0x65,
+        0x34, // Decodes as 1CECHNKREP0F1RSTCMT0
+    ]);
 
     pub fn from_buffer(buffer: Vec<u8>) -> IcechunkResult<Snapshot> {
         let _ = flatbuffers::root_with_opts::<generated::Snapshot>(
@@ -429,7 +433,7 @@ impl Snapshot {
         let properties = [("__root".to_string(), serde_json::Value::from(true))].into();
         let nodes: Vec<Result<NodeSnapshot, Infallible>> = Vec::new();
         Self::from_iter(
-            None,
+            Some(Self::INITIAL_SNAPSHOT_ID),
             None,
             Self::INITIAL_COMMIT_MESSAGE.to_string(),
             Some(properties),

icechunk/src/ops/gc.rs

@@ -14,7 +14,7 @@ use crate::{
     },
     ops::pointed_snapshots,
     refs::{Ref, RefError, delete_branch, delete_tag, list_refs},
-    repository::RepositoryError,
+    repository::{RepositoryError, RepositoryErrorKind},
     storage::{self, DeleteObjectsResult, ListInfo},
 };
 
@@ -160,6 +160,13 @@ pub async fn garbage_collect(
     asset_manager: Arc<AssetManager>,
     config: &GCConfig,
 ) -> GCResult<GCSummary> {
+    if !storage.can_write() {
+        return Err(GCError::Repository(
+            RepositoryErrorKind::ReadonlyStorage("Cannot garbage collect".to_string())
+                .into(),
+        ));
+    }
+
     // TODO: this function could have much more parallelism
     if !config.action_needed() {
         tracing::info!("No action requested");
@@ -543,6 +550,13 @@ pub async fn expire(
     expired_branches: ExpiredRefAction,
     expired_tags: ExpiredRefAction,
 ) -> GCResult<ExpireResult> {
+    if !storage.can_write() {
+        return Err(GCError::Repository(
+            RepositoryErrorKind::ReadonlyStorage("Cannot expire snapshots".to_string())
+                .into(),
+        ));
+    }
+
     let all_refs = stream::iter(list_refs(storage, storage_settings).await?);
     let asset_manager = Arc::clone(&asset_manager.clone());
 

icechunk/src/session.rs

@@ -3623,6 +3623,7 @@ mod tests {
         assert_eq!(parents[0].message, "second commit");
         assert_eq!(parents[1].message, "first commit");
         assert_eq!(parents[2].message, Snapshot::INITIAL_COMMIT_MESSAGE);
+        assert_eq!(parents[2].id, Snapshot::INITIAL_SNAPSHOT_ID);
         itertools::assert_equal(
             parents.iter().sorted_by_key(|m| m.flushed_at).rev(),
             parents.iter(),
@@ -3692,6 +3693,7 @@ mod tests {
         assert!(msg == "from 1" || msg == "from 2");
 
         assert_eq!(parents[1].message.as_str(), Snapshot::INITIAL_COMMIT_MESSAGE);
+        assert_eq!(parents[1].id, Snapshot::INITIAL_SNAPSHOT_ID);
         Ok(())
     }