Conversation
62b508f to
c3d0334
Compare
a7073eb to
6333236
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements upload packing for the S3d Sia backend, allowing small objects to be batched together into packed uploads to minimize storage overhead on the Sia network. The Packer uses a timer-based batching strategy: small objects are accumulated in a batch until the slab is full or a 1-second flush timeout fires, whichever comes first.
Changes:
- Introduces a new
Packertype (sia/packing.go) that batches small objects into packed uploads, with timer-based flushing and concurrent-safe access - Extends the
SDKinterface and its implementations (IndexdSDK,MemorySDK) withUploadPacked()andSaveObject()methods needed for packed upload support - Integrates packing into
PutObjectflow, routing small objects through the packer while large objects continue using direct upload
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sia/packing.go | New file implementing the Packer type with batching, timer-based flushing, and concurrent upload packing |
| sia/packing_test.go | Comprehensive tests for the packer including basic operations, flush behavior, close semantics, and concurrency |
| sia/objects.go | Routes small objects through the packer in PutObject based on NeedsPacking check |
| sia/sia.go | Adds Packer field to Sia struct, WithPacker option, and Close() method for cleanup |
| sia/sdk.go | Adds UploadPacked() and SaveObject() to IndexdSDK |
| sia/sia_test.go | Updates MemorySDK mock to implement new SDK interface methods; adds Close cleanup |
| go.mod | Updates Go version, adds replace directive for local indexd, updates dependencies |
| go.sum | Reflects dependency changes (coreutils update, indexd removal, time addition) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b16c449 to
926994a
Compare
926994a to
5532dab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5532dab to
605ca4e
Compare
605ca4e to
caab1cf
Compare
…d into pj/upload-packing
Before (old
|
| # | Objects | Size (MB) | Slabs | Waste |
|---|---|---|---|---|
| 1 | 5 | 159.8 | 4 | 0.13% |
| 2 | 6 | 199.9 | 5 | 0.05% |
| 3 | 5 | 199.8 | 5 | 0.10% |
| 4 | 6 | 239.6 | 6 | 0.14% |
| 5 | 6 | 239.5 | 6 | 0.20% |
| 6 | 9 | 359.9 | 9 | 0.02% |
| 7 | 2 | 39.6 | 1 | 0.94% |
| 8 | 1 | 38.2 | 1 | 4.57% |
| 9 | 1 | 38.1 | 1 | 4.76% |
| 10 | 2 | 39.7 | 1 | 0.74% |
| 11 | 1 | 37.1 | 1 | 7.29% |
| 12 | 1 | 36.8 | 1 | 8.11% |
| 13 | 1 | 36.6 | 1 | 8.51% |
| 14 | 1 | 36.3 | 1 | 9.20% |
| 15 | 2 | 39.9 | 1 | 0.13% |
| 16 | 7 | 238.7 | 6 | 0.55% |
| 17 | 7 | 199.8 | 5 | 0.12% |
| 18 | 4 | 117.1 | 3 | 2.37% |
| 19 | 5 | 118.4 | 3 | 1.29% |
| 20 | 5 | 119.2 | 3 | 0.65% |
| 21 | 4 | 79.3 | 2 | 0.84% |
| 22 | 4 | 79.3 | 2 | 0.84% |
| 23 | 8 | 158.9 | 4 | 0.69% |
| 24 | 8 | 116.4 | 3 | 2.98% |
| 25 | 6 | 79.1 | 2 | 1.13% |
After (new tryAdd logic with maxSlabsPerPack=6)
| # | Objects | Size (MB) | Slabs | Waste |
|---|---|---|---|---|
| 1 | 6 | 238.9 | 6 | 0.42% |
| 2 | 6 | 239.8 | 6 | 0.06% |
| 3 | 6 | 239.5 | 6 | 0.21% |
| 4 | 6 | 239.4 | 6 | 0.22% |
| 5 | 7 | 239.7 | 6 | 0.13% |
| 6 | 7 | 239.9 | 6 | 0.02% |
| 7 | 8 | 239.5 | 6 | 0.19% |
| 8 | 7 | 199.8 | 5 | 0.08% |
| 9 | 6 | 119.6 | 3 | 0.36% |
| 10 | 4 | 79.9 | 2 | 0.08% |
| 11 | 4 | 79.8 | 2 | 0.26% |
| 12 | 6 | 119.9 | 3 | 0.06% |
| 13 | 6 | 119.2 | 3 | 0.66% |
| 14 | 8 | 158.7 | 4 | 0.84% |
| 15 | 4 | 79.3 | 2 | 0.88% |
| 16 | 2 | 38.0 | 1 | 5.06% |
| 17 | 10 | 159.4 | 4 | 0.34% |
| 18 | 5 | 78.9 | 2 | 1.36% |
| 19 | 7 | 79.1 | 2 | 1.06% |
| 20 | 4 | 38.4 | 1 | 4.08% |
| 21 | 5 | 39.8 | 1 | 0.41% |
Summary
| Before | After | Change | |
|---|---|---|---|
| Objects packed | 111 | 130 | +19 |
| Packs | 25 | 21 | 4 fewer |
| Total slabs | 78 | 83 | +5 (more objects) |
| Max waste | 9.20% | 5.06% | down 45% |
| Packs with >4% waste | 6 | 2 | down 67% |
| Single-object packs | 6 | 0 | eliminated |
| Leftover objects | 4 (stuck) | 6 | similar |
| Max slabs per pack | 9 | 6 | capped |
@ChrisSchinnerl I want to update the packing algorithm and re-introduce maxSlabsPerPack. Even though it's an arbitrary number it's meant to ensure we get all benefits from parallelising the uploads, as well as prevent single slab objects with high waste pct. It's a minor (so cheap) change with generally a lot better results so we should add it back in (?)
This PR adds upload packing.
Closes #100