feat: concurrent manifest downloads in ListSnapshots #50

2026-03-18T05:52:05+01:00

clawbot okomentoval

2026-03-18 05:52:05 +01:00

Spolupracovník

Summary

Replace serial getManifestSize() calls in ListSnapshots with bounded concurrent downloads using errgroup. For each remote snapshot not in the local DB, manifest downloads now run in parallel (up to 10 concurrent goroutines) instead of one at a time.

Changes

Use errgroup with SetLimit(10) for bounded concurrency
Collect remote-only snapshot IDs first, pre-add entries with zero size
Download manifests concurrently, patch sizes from results
Remove now-unused getManifestSize helper (logic inlined into goroutines)
Promote golang.org/x/sync from indirect to direct dependency

Testing

make check passes (fmt-check, lint, tests)
docker build . passes

closes #8

## Summary Replace serial `getManifestSize()` calls in `ListSnapshots` with bounded concurrent downloads using `errgroup`. For each remote snapshot not in the local DB, manifest downloads now run in parallel (up to 10 concurrent goroutines) instead of one at a time. ## Changes - Use `errgroup` with `SetLimit(10)` for bounded concurrency - Collect remote-only snapshot IDs first, pre-add entries with zero size - Download manifests concurrently, patch sizes from results - Remove now-unused `getManifestSize` helper (logic inlined into goroutines) - Promote `golang.org/x/sync` from indirect to direct dependency ## Testing - `make check` passes (fmt-check, lint, tests) - `docker build .` passes closes https://git.eeqj.de/sneak/vaultik/issues/8

clawbot přidal/a 1 commit 2026-03-18 05:52:05 +01:00

feat: concurrent manifest downloads in ListSnapshots

check / check (pull_request) Successful in 4m7s

Podrobnosti

588e84da9c

Replace serial getManifestSize() calls with bounded concurrent downloads
using errgroup. For each remote snapshot not in the local DB, manifest
downloads now run in parallel (up to 10 concurrent) instead of one at a
time.

Changes:
- Use errgroup with SetLimit(10) for bounded concurrency
- Collect remote-only snapshot IDs first, pre-add entries with zero size
- Download manifests concurrently, patch sizes from results
- Remove now-unused getManifestSize helper (logic inlined into goroutines)
- Promote golang.org/x/sync from indirect to direct dependency

closes #8

clawbot přidal/a needs-review štítek 2026-03-18 05:52:44 +01:00

clawbot okomentoval

2026-03-18 05:56:52 +01:00

Autor

Spolupracovník

Review: PASS ✅

Reviewed PR #50 — concurrent manifest downloads for issue #8.

Checklist

✅ errgroup pattern: errgroup.WithContext(v.ctx) + g.SetLimit(maxConcurrentManifestDownloads) — correct bounded concurrency
✅ Synchronization: sync.Mutex protects shared results slice; snapshots slice is only mutated single-threaded (before/after errgroup)
✅ getManifestSize removed: Fully deleted. Logic inlined into goroutines using gctx (errgroup context) instead of v.ctx for proper cancellation — correct design choice over reusing downloadManifest which uses v.ctx
✅ No magic numbers: const maxConcurrentManifestDownloads = 10 with descriptive comment
✅ golang.org/x/sync: Promoted from indirect → direct in go.mod; hash-pinned in go.sum
✅ Scope: Only go.mod and internal/vaultik/snapshot.go changed — no linter config, test, or Makefile modifications
✅ Go 1.26.1: Range variable capture is per-iteration — no closure bug
✅ Error propagation: Single goroutine failure cancels context and returns error, matching original serial behavior
✅ docker build .: All stages pass (lint, fmt-check, tests, build)

Clean implementation. No issues found.

## Review: PASS ✅ Reviewed [PR #50](https://git.eeqj.de/sneak/vaultik/pulls/50) — concurrent manifest downloads for [issue #8](https://git.eeqj.de/sneak/vaultik/issues/8). ### Checklist - ✅ **errgroup pattern**: `errgroup.WithContext(v.ctx)` + `g.SetLimit(maxConcurrentManifestDownloads)` — correct bounded concurrency - ✅ **Synchronization**: `sync.Mutex` protects shared `results` slice; `snapshots` slice is only mutated single-threaded (before/after errgroup) - ✅ **`getManifestSize` removed**: Fully deleted. Logic inlined into goroutines using `gctx` (errgroup context) instead of `v.ctx` for proper cancellation — correct design choice over reusing `downloadManifest` which uses `v.ctx` - ✅ **No magic numbers**: `const maxConcurrentManifestDownloads = 10` with descriptive comment - ✅ **golang.org/x/sync**: Promoted from indirect → direct in `go.mod`; hash-pinned in `go.sum` - ✅ **Scope**: Only `go.mod` and `internal/vaultik/snapshot.go` changed — no linter config, test, or Makefile modifications - ✅ **Go 1.26.1**: Range variable capture is per-iteration — no closure bug - ✅ **Error propagation**: Single goroutine failure cancels context and returns error, matching original serial behavior - ✅ **`docker build .`**: All stages pass (lint, fmt-check, tests, build) Clean implementation. No issues found.

clawbot přidáno merge-ready a odebráno needs-review štítků 2026-03-18 05:58:06 +01:00

sneak byl přiřazen clawbot

2026-03-18 05:58:06 +01:00

clawbot vynucené nahrání fix/concurrent-manifest-downloads od 588e84da9c do d39d939c5b

2026-03-20 06:55:47 +01:00

Porovnat

clawbot okomentoval

2026-03-20 06:55:54 +01:00

Autor

Spolupracovník

Rebased fix/concurrent-manifest-downloads onto current main (after #39, #41, #49, #55 merges).

Conflict was in internal/vaultik/snapshot.go — main had refactored ListSnapshots into extracted helper methods (buildSnapshotInfoList). Resolved by applying the concurrent errgroup pattern within the new function structure. Also fixed a return-value arity mismatch introduced by the rebase (return err → return nil, err).

docker build . passes: fmt-check ✅, lint (0 issues) ✅, all tests ✅, build ✅.

Rebased `fix/concurrent-manifest-downloads` onto current `main` (after [#39](https://git.eeqj.de/sneak/vaultik/pulls/39), [#41](https://git.eeqj.de/sneak/vaultik/pulls/41), [#49](https://git.eeqj.de/sneak/vaultik/pulls/49), [#55](https://git.eeqj.de/sneak/vaultik/pulls/55) merges). Conflict was in `internal/vaultik/snapshot.go` — main had refactored `ListSnapshots` into extracted helper methods (`buildSnapshotInfoList`). Resolved by applying the concurrent errgroup pattern within the new function structure. Also fixed a return-value arity mismatch introduced by the rebase (`return err` → `return nil, err`). `docker build .` passes: fmt-check ✅, lint (0 issues) ✅, all tests ✅, build ✅.

clawbot přidáno needs-review a odebráno merge-ready štítků 2026-03-20 06:58:55 +01:00

clawbot okomentoval

2026-03-20 07:00:27 +01:00

Autor

Spolupracovník

Review: PASS ✅ (post-rebase)

Reviewed PR #50 — concurrent manifest downloads for issue #8, rebased onto current main.

Checklist

✅ Rebase conflict resolution: Concurrent errgroup pattern correctly placed within buildSnapshotInfoList (the extracted helper from the refactored ListSnapshots). Return-value arity (return nil, err) matches the ([]SnapshotInfo, error) signature — no mismatches.
✅ errgroup pattern: errgroup.WithContext(v.ctx) + g.SetLimit(maxConcurrentManifestDownloads) — bounded concurrency with proper parent context derivation
✅ Synchronization: sync.Mutex protects shared results slice; snapshots slice only mutated single-threaded (before/after errgroup)
✅ Context usage: Goroutines use gctx (errgroup-derived context) for v.Storage.Get(), not v.ctx — cancellation propagates correctly on first error
✅ getManifestSize removed: Fully deleted, zero references remain. Logic inlined into goroutines.
✅ No magic numbers: const maxConcurrentManifestDownloads = 10 with descriptive comment
✅ golang.org/x/sync: Promoted from indirect → direct in go.mod
✅ Scope: Only go.mod and internal/vaultik/snapshot.go changed — no linter config, test, or Makefile modifications
✅ Go 1.26.1: Range variable sid captured per-iteration — no closure bug
✅ Error propagation: g.Wait() returns first error, cancelling remaining goroutines via gctx
✅ docker build .: All stages pass (fmt-check ✅, lint ✅, tests ✅, build ✅)

Clean rebase. No issues found.

## Review: PASS ✅ (post-rebase) Reviewed [PR #50](https://git.eeqj.de/sneak/vaultik/pulls/50) — concurrent manifest downloads for [issue #8](https://git.eeqj.de/sneak/vaultik/issues/8), rebased onto current `main`. ### Checklist - ✅ **Rebase conflict resolution**: Concurrent errgroup pattern correctly placed within `buildSnapshotInfoList` (the extracted helper from the refactored `ListSnapshots`). Return-value arity (`return nil, err`) matches the `([]SnapshotInfo, error)` signature — no mismatches. - ✅ **errgroup pattern**: `errgroup.WithContext(v.ctx)` + `g.SetLimit(maxConcurrentManifestDownloads)` — bounded concurrency with proper parent context derivation - ✅ **Synchronization**: `sync.Mutex` protects shared `results` slice; `snapshots` slice only mutated single-threaded (before/after errgroup) - ✅ **Context usage**: Goroutines use `gctx` (errgroup-derived context) for `v.Storage.Get()`, not `v.ctx` — cancellation propagates correctly on first error - ✅ **`getManifestSize` removed**: Fully deleted, zero references remain. Logic inlined into goroutines. - ✅ **No magic numbers**: `const maxConcurrentManifestDownloads = 10` with descriptive comment - ✅ **golang.org/x/sync**: Promoted from indirect → direct in `go.mod` - ✅ **Scope**: Only `go.mod` and `internal/vaultik/snapshot.go` changed — no linter config, test, or Makefile modifications - ✅ **Go 1.26.1**: Range variable `sid` captured per-iteration — no closure bug - ✅ **Error propagation**: `g.Wait()` returns first error, cancelling remaining goroutines via `gctx` - ✅ **`docker build .`**: All stages pass (fmt-check ✅, lint ✅, tests ✅, build ✅) Clean rebase. No issues found.

clawbot přidáno merge-ready a odebráno needs-review štítků 2026-03-20 07:01:13 +01:00

sneak sloučený commit dcf3ec399a do main

2026-03-22 00:41:40 +01:00

sneak odstranil/a větev fix/concurrent-manifest-downloads

2026-03-22 00:41:40 +01:00

sneak odkázal na tento úkol z commitu

2026-03-22 00:41:41 +01:00

feat: concurrent manifest downloads in ListSnapshots (#50)

Přihlaste se pro zapojení do konverzace.

1 účastníků

Oznámení

Termín dokončení

Žádný termín dokončení.

Závislosti

Nejsou nastaveny žádné závislosti.

Reference: sneak/vaultik#50