This is an automated email from the ASF dual-hosted git repository.

zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git


The following commit(s) were added to refs/heads/main by this push:
     new a5a06651b perf(table): prefetch historical snapshot manifest lists 
(#2234)
a5a06651b is described below

commit a5a06651b76d8dfe378b8f9260d84c3d3358cd50
Author: Minh Vu <[email protected]>
AuthorDate: Tue Oct 20 19:29:08 2026 +0200

    perf(table): prefetch historical snapshot manifest lists (#2234)
    
    * perf(table): prefetch historical snapshot manifest lists
    
    * perf(table): keep warm manifest scans synchronous
---
 table/inspect_files.go                  |  24 +--
 table/inspect_snapshot_prefetch.go      | 225 ++++++++++++++++++++
 table/inspect_snapshot_prefetch_test.go | 360 ++++++++++++++++++++++++++++++++
 table/snapshot_manifest_cache.go        |  19 ++
 4 files changed, 605 insertions(+), 23 deletions(-)

diff --git a/table/inspect_files.go b/table/inspect_files.go
index 4352d34d0..42b7915fc 100644
--- a/table/inspect_files.go
+++ b/table/inspect_files.go
@@ -172,29 +172,7 @@ func (i InspectTable) allManifestEntryReader(
        return i.manifestEntryReaderFromManifestSource(
                ctx, arrowSchema, fs, discardDeleted, includeManifest, 
appendEntry,
                func(yield func(iceberg.ManifestFile, error) bool) {
-                       seen := make(map[string]struct{})
-                       for _, snapshot := range snapshots {
-                               if err := ctx.Err(); err != nil {
-                                       yield(nil, err)
-
-                                       return
-                               }
-                               manifests, err := provider.Manifests(ctx, 
snapshot)
-                               if err != nil {
-                                       yield(nil, fmt.Errorf("read snapshot %d 
manifests: %w", snapshot.SnapshotID, err))
-
-                                       return
-                               }
-                               for manifest := range manifests {
-                                       if _, ok := seen[manifest.FilePath()]; 
ok {
-                                               continue
-                                       }
-                                       seen[manifest.FilePath()] = struct{}{}
-                                       if !yield(manifest, nil) {
-                                               return
-                                       }
-                               }
-                       }
+                       streamSnapshotManifests(ctx, snapshots, 
provider.cachedSet, provider.set, yield)
                }), nil
 }
 
diff --git a/table/inspect_snapshot_prefetch.go 
b/table/inspect_snapshot_prefetch.go
new file mode 100644
index 000000000..10a2b8378
--- /dev/null
+++ b/table/inspect_snapshot_prefetch.go
@@ -0,0 +1,225 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package table
+
+import (
+       "context"
+       "fmt"
+       "sync"
+
+       "github.com/apache/iceberg-go"
+)
+
+const inspectSnapshotManifestPrefetch = 8
+
+type inspectSnapshotManifestRead struct {
+       set snapshotManifestSet
+       err error
+}
+
+// streamSnapshotManifests loads historical manifest lists concurrently while
+// yielding them in snapshot order. Only a small window of lists can be held
+// by in-flight reads, even for tables with many snapshots.
+func streamSnapshotManifests(
+       ctx context.Context,
+       snapshots []Snapshot,
+       cached func(Snapshot) (snapshotManifestSet, bool),
+       load func(context.Context, Snapshot) (snapshotManifestSet, error),
+       yield func(iceberg.ManifestFile, error) bool,
+) {
+       if len(snapshots) == 0 {
+               return
+       }
+       if err := ctx.Err(); err != nil {
+               yield(nil, err)
+
+               return
+       }
+       seen := make(map[string]struct{})
+       if len(snapshots) == 1 {
+               set, ok := snapshotManifestSet{}, false
+               if cached != nil {
+                       set, ok = cached(snapshots[0])
+               }
+               if !ok {
+                       var err error
+                       set, err = load(ctx, snapshots[0])
+                       if err != nil {
+                               yield(nil, fmt.Errorf("read snapshot %d 
manifests: %w", snapshots[0].SnapshotID, err))
+
+                               return
+                       }
+               }
+               for _, manifest := range set.borrowAllManifests() {
+                       path := manifest.FilePath()
+                       if _, ok := seen[path]; ok {
+                               continue
+                       }
+                       seen[path] = struct{}{}
+                       if !yield(manifest, nil) {
+                               return
+                       }
+               }
+
+               return
+       }
+       if cached != nil {
+               start := 0
+               for start < len(snapshots) {
+                       count := min(inspectSnapshotManifestPrefetch, 
len(snapshots)-start)
+                       var sets 
[inspectSnapshotManifestPrefetch]snapshotManifestSet
+                       allCached := true
+                       for i := range count {
+                               set, ok := cached(snapshots[start+i])
+                               if !ok {
+                                       allCached = false
+
+                                       break
+                               }
+                               sets[i] = set
+                       }
+                       if !allCached {
+                               break
+                       }
+
+                       for i := range count {
+                               if err := ctx.Err(); err != nil {
+                                       yield(nil, err)
+
+                                       return
+                               }
+                               for _, manifest := range 
sets[i].borrowAllManifests() {
+                                       path := manifest.FilePath()
+                                       if _, ok := seen[path]; ok {
+                                               continue
+                                       }
+                                       seen[path] = struct{}{}
+                                       if !yield(manifest, nil) {
+                                               return
+                                       }
+                               }
+                       }
+                       start += count
+               }
+               if start == len(snapshots) {
+                       return
+               }
+               // The first cache miss starts the concurrent path. Recheck 
this small
+               // window there so cached and newly loaded results share one 
ordered path.
+               snapshots = snapshots[start:]
+       }
+
+       readCtx := ctx
+       var cancel context.CancelFunc
+       var workers sync.WaitGroup
+       // Cancel first, then wait for all readers before returning. This also
+       // handles a consumer that stops iterating partway through a snapshot.
+       defer func() {
+               if cancel != nil {
+                       cancel()
+               }
+               workers.Wait()
+       }()
+
+       window := min(len(snapshots), inspectSnapshotManifestPrefetch)
+       results := make([]chan inspectSnapshotManifestRead, window)
+       ready := make([]bool, window)
+       readyResults := make([]inspectSnapshotManifestRead, window)
+
+       startRead := func(index int) {
+               if cancel == nil {
+                       readCtx, cancel = context.WithCancel(ctx)
+               }
+               slot := index % window
+               if results[slot] == nil {
+                       // Buffer one result so a cancelled worker never blocks 
sending it.
+                       results[slot] = make(chan inspectSnapshotManifestRead, 
1)
+               }
+               ready[slot] = false
+               workers.Go(func() {
+                       set, err := load(readCtx, snapshots[index])
+                       results[slot] <- inspectSnapshotManifestRead{set: set, 
err: err}
+               })
+       }
+
+       for i := range window {
+               if cached != nil {
+                       if set, ok := cached(snapshots[i]); ok {
+                               ready[i] = true
+                               readyResults[i] = 
inspectSnapshotManifestRead{set: set}
+
+                               continue
+                       }
+               }
+               startRead(i)
+       }
+
+       for i, snapshot := range snapshots {
+               if err := readCtx.Err(); err != nil {
+                       yield(nil, err)
+
+                       return
+               }
+
+               slot := i % window
+               var result inspectSnapshotManifestRead
+               if ready[slot] {
+                       result = readyResults[slot]
+                       ready[slot] = false
+               } else {
+                       select {
+                       case <-readCtx.Done():
+                               yield(nil, readCtx.Err())
+
+                               return
+                       case result = <-results[slot]:
+                       }
+               }
+
+               if result.err != nil {
+                       yield(nil, fmt.Errorf("read snapshot %d manifests: %w", 
snapshot.SnapshotID, result.err))
+
+                       return
+               }
+
+               // Reuse the completed slot before yielding. The consumer may 
spend a
+               // long time decoding these manifests while the next list is 
fetched.
+               if next := i + window; next < len(snapshots) {
+                       if cached != nil {
+                               if set, ok := cached(snapshots[next]); ok {
+                                       ready[slot] = true
+                                       readyResults[slot] = 
inspectSnapshotManifestRead{set: set}
+                               } else {
+                                       startRead(next)
+                               }
+                       } else {
+                               startRead(next)
+                       }
+               }
+
+               for _, manifest := range result.set.borrowAllManifests() {
+                       path := manifest.FilePath()
+                       if _, ok := seen[path]; ok {
+                               continue
+                       }
+                       seen[path] = struct{}{}
+                       if !yield(manifest, nil) {
+                               return
+                       }
+               }
+       }
+}
diff --git a/table/inspect_snapshot_prefetch_test.go 
b/table/inspect_snapshot_prefetch_test.go
new file mode 100644
index 000000000..990a60493
--- /dev/null
+++ b/table/inspect_snapshot_prefetch_test.go
@@ -0,0 +1,360 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package table
+
+import (
+       "context"
+       "errors"
+       "fmt"
+       "slices"
+       "sync/atomic"
+       "testing"
+       "time"
+
+       "github.com/apache/iceberg-go"
+       "github.com/stretchr/testify/require"
+)
+
+func TestInspectSnapshotManifestPrefetchPreservesOrderAndBoundsReads(t 
*testing.T) {
+       snapshots := make([]Snapshot, 25)
+       for i := range snapshots {
+               snapshots[i].SnapshotID = int64(i + 1)
+       }
+
+       ctx := t.Context()
+       started := make(chan int64, inspectSnapshotManifestPrefetch)
+       release := make(chan struct{})
+       defer func() {
+               select {
+               case <-release:
+               default:
+                       close(release)
+               }
+       }()
+
+       var active, maxActive atomic.Int32
+       resultCh := make(chan []string, 1)
+       go func() {
+               var paths []string
+               streamSnapshotManifests(ctx, snapshots, nil,
+                       func(ctx context.Context, snapshot Snapshot) 
(snapshotManifestSet, error) {
+                               current := active.Add(1)
+                               defer active.Add(-1)
+                               for {
+                                       previous := maxActive.Load()
+                                       if current <= previous || 
maxActive.CompareAndSwap(previous, current) {
+                                               break
+                                       }
+                               }
+
+                               if snapshot.SnapshotID <= 
inspectSnapshotManifestPrefetch {
+                                       started <- snapshot.SnapshotID
+                               }
+                               select {
+                               case <-release:
+                               case <-ctx.Done():
+                                       return snapshotManifestSet{}, ctx.Err()
+                               }
+
+                               path := fmt.Sprintf("manifest-%d.avro", 
snapshot.SnapshotID)
+                               manifest := iceberg.NewManifestFile(2, path, 
10, 0, 1).Build()
+
+                               return snapshotManifestSet{all: 
[]iceberg.ManifestFile{manifest}}, nil
+                       },
+                       func(manifest iceberg.ManifestFile, err error) bool {
+                               if err != nil {
+                                       paths = append(paths, "ERROR: 
"+err.Error())
+
+                                       return false
+                               }
+                               paths = append(paths, manifest.FilePath())
+
+                               return true
+                       })
+               resultCh <- paths
+       }()
+
+       var firstWindow []int64
+       for range inspectSnapshotManifestPrefetch {
+               select {
+               case id := <-started:
+                       firstWindow = append(firstWindow, id)
+               case <-time.After(5 * time.Second):
+                       t.Fatal("initial manifest reads did not start 
concurrently")
+               }
+       }
+       slices.Sort(firstWindow)
+       for i, id := range firstWindow {
+               require.Equal(t, int64(i+1), id)
+       }
+       require.EqualValues(t, inspectSnapshotManifestPrefetch, 
maxActive.Load())
+       close(release)
+
+       select {
+       case paths := <-resultCh:
+               require.Len(t, paths, len(snapshots))
+               for i, path := range paths {
+                       require.Equal(t, fmt.Sprintf("manifest-%d.avro", i+1), 
path)
+               }
+       case <-time.After(5 * time.Second):
+               t.Fatal("manifest prefetch failed to finish")
+       }
+       require.EqualValues(t, 0, active.Load())
+       require.LessOrEqual(t, maxActive.Load(), 
int32(inspectSnapshotManifestPrefetch))
+}
+
+func TestInspectSnapshotManifestPrefetchUsesCachedSets(t *testing.T) {
+       snapshots := make([]Snapshot, 12)
+       cachedSets := make(map[int64]snapshotManifestSet)
+       for i := range snapshots {
+               snapshots[i].SnapshotID = int64(i + 1)
+               if i%2 == 0 {
+                       path := fmt.Sprintf("manifest-%d.avro", i+1)
+                       manifest := iceberg.NewManifestFile(2, path, 10, 0, 
1).Build()
+                       cachedSets[int64(i+1)] = snapshotManifestSet{all: 
[]iceberg.ManifestFile{manifest}}
+               }
+       }
+
+       var loaded atomic.Int32
+       var paths []string
+       streamSnapshotManifests(context.Background(), snapshots,
+               func(snapshot Snapshot) (snapshotManifestSet, bool) {
+                       set, ok := cachedSets[snapshot.SnapshotID]
+
+                       return set, ok
+               },
+               func(_ context.Context, snapshot Snapshot) 
(snapshotManifestSet, error) {
+                       loaded.Add(1)
+                       path := fmt.Sprintf("manifest-%d.avro", 
snapshot.SnapshotID)
+                       manifest := iceberg.NewManifestFile(2, path, 10, 0, 
1).Build()
+
+                       return snapshotManifestSet{all: 
[]iceberg.ManifestFile{manifest}}, nil
+               },
+               func(manifest iceberg.ManifestFile, err error) bool {
+                       require.NoError(t, err)
+                       paths = append(paths, manifest.FilePath())
+
+                       return true
+               })
+
+       require.EqualValues(t, len(snapshots)/2, loaded.Load())
+       require.Len(t, paths, len(snapshots))
+       for i, path := range paths {
+               require.Equal(t, fmt.Sprintf("manifest-%d.avro", i+1), path)
+       }
+}
+
+func TestInspectSnapshotManifestPrefetchDeduplicatesPaths(t *testing.T) {
+       snapshots := []Snapshot{{SnapshotID: 1}, {SnapshotID: 2}, {SnapshotID: 
3}}
+       manifests := [][]string{{"a.avro", "shared.avro"}, {"shared.avro", 
"b.avro"}, {"a.avro", "c.avro"}}
+       var paths []string
+       streamSnapshotManifests(context.Background(), snapshots, nil,
+               func(_ context.Context, snapshot Snapshot) 
(snapshotManifestSet, error) {
+                       set := snapshotManifestSet{}
+                       for _, path := range manifests[snapshot.SnapshotID-1] {
+                               set.all = append(set.all, 
iceberg.NewManifestFile(2, path, 10, 0, 1).Build())
+                       }
+
+                       return set, nil
+               },
+               func(manifest iceberg.ManifestFile, err error) bool {
+                       require.NoError(t, err)
+                       paths = append(paths, manifest.FilePath())
+
+                       return true
+               })
+
+       require.Equal(t, []string{"a.avro", "shared.avro", "b.avro", "c.avro"}, 
paths)
+}
+
+func TestInspectSnapshotManifestPrefetchStopsReadersOnEarlyExit(t *testing.T) {
+       snapshots := make([]Snapshot, 30)
+       for i := range snapshots {
+               snapshots[i].SnapshotID = int64(i + 1)
+       }
+
+       var active atomic.Int32
+       var yielded int
+       streamSnapshotManifests(context.Background(), snapshots, nil,
+               func(ctx context.Context, snapshot Snapshot) 
(snapshotManifestSet, error) {
+                       active.Add(1)
+                       defer active.Add(-1)
+
+                       if snapshot.SnapshotID == 1 {
+                               manifest := iceberg.NewManifestFile(2, 
"first.avro", 10, 0, 1).Build()
+
+                               return snapshotManifestSet{all: 
[]iceberg.ManifestFile{manifest}}, nil
+                       }
+                       <-ctx.Done()
+
+                       return snapshotManifestSet{}, ctx.Err()
+               },
+               func(_ iceberg.ManifestFile, _ error) bool {
+                       yielded++
+
+                       return false
+               })
+       require.Equal(t, 1, yielded)
+       require.EqualValues(t, 0, active.Load(), "all workers must finish 
before return")
+}
+
+func TestInspectSnapshotManifestPrefetchReportsFirstOrderedError(t *testing.T) 
{
+       failure := errors.New("manifest load failed")
+       snapshots := []Snapshot{{SnapshotID: 1}, {SnapshotID: 2}, {SnapshotID: 
3}}
+       var paths []string
+       var gotErr error
+
+       streamSnapshotManifests(context.Background(), snapshots, nil,
+               func(_ context.Context, snapshot Snapshot) 
(snapshotManifestSet, error) {
+                       if snapshot.SnapshotID == 2 {
+                               return snapshotManifestSet{}, failure
+                       }
+                       path := fmt.Sprintf("manifest-%d.avro", 
snapshot.SnapshotID)
+                       manifest := iceberg.NewManifestFile(2, path, 10, 0, 
1).Build()
+
+                       return snapshotManifestSet{all: 
[]iceberg.ManifestFile{manifest}}, nil
+               },
+               func(manifest iceberg.ManifestFile, err error) bool {
+                       if err != nil {
+                               gotErr = err
+
+                               return false
+                       }
+                       paths = append(paths, manifest.FilePath())
+
+                       return true
+               })
+
+       require.Equal(t, []string{"manifest-1.avro"}, paths)
+       require.ErrorIs(t, gotErr, failure)
+       require.ErrorContains(t, gotErr, "snapshot 2")
+}
+
+func BenchmarkInspectSnapshotManifestListPrefetch(b *testing.B) {
+       const snapshotCount = 64
+       snapshots := make([]Snapshot, snapshotCount)
+       for i := range snapshots {
+               snapshots[i].SnapshotID = int64(i + 1)
+       }
+
+       // Model an object-store manifest-list read with 1 ms of latency. This
+       // measures the I/O overlap, not end-to-end metadata-table throughput.
+       load := func(ctx context.Context, _ Snapshot) (snapshotManifestSet, 
error) {
+               select {
+               case <-time.After(time.Millisecond):
+                       return snapshotManifestSet{}, nil
+               case <-ctx.Done():
+                       return snapshotManifestSet{}, ctx.Err()
+               }
+       }
+
+       b.Run("sequential", func(b *testing.B) {
+               b.ReportAllocs()
+               b.ReportMetric(snapshotCount, "lists/op")
+               for b.Loop() {
+                       for _, snapshot := range snapshots {
+                               if _, err := load(b.Context(), snapshot); err 
!= nil {
+                                       b.Fatal(err)
+                               }
+                       }
+               }
+       })
+       b.Run("prefetch8", func(b *testing.B) {
+               b.ReportAllocs()
+               b.ReportMetric(snapshotCount, "lists/op")
+               for b.Loop() {
+                       streamSnapshotManifests(b.Context(), snapshots, nil, 
load,
+                               func(_ iceberg.ManifestFile, err error) bool {
+                                       if err != nil {
+                                               b.Fatal(err)
+                                       }
+
+                                       return true
+                               })
+               }
+       })
+}
+
+func BenchmarkInspectSnapshotManifestListCacheHit(b *testing.B) {
+       cache := newSnapshotManifestCache()
+
+       for _, count := range []int{1, 64} {
+               b.Run(fmt.Sprintf("snapshots=%d", count), func(b *testing.B) {
+                       snapshots := make([]Snapshot, count)
+                       sets := make([]snapshotManifestSet, count)
+                       for i := range snapshots {
+                               snapshots[i].SnapshotID = int64(i + 1)
+                               manifest := iceberg.NewManifestFile(2, 
fmt.Sprintf("manifest-%d.avro", i+1), 10, 0, 1).Build()
+                               sets[i] = snapshotManifestSet{all: 
[]iceberg.ManifestFile{manifest}}
+                               _, err := cache.get(context.Background(), 
snapshots[i], func(context.Context) (snapshotManifestSet, error) {
+                                       return sets[i], nil
+                               })
+                               if err != nil {
+                                       b.Fatal(err)
+                               }
+                       }
+
+                       load := func(_ context.Context, snapshot Snapshot) 
(snapshotManifestSet, error) {
+                               return sets[snapshot.SnapshotID-1], nil
+                       }
+                       b.Run("sequential", func(b *testing.B) {
+                               b.ReportAllocs()
+                               for b.Loop() {
+                                       var yielded int
+                                       seen := make(map[string]struct{})
+                                       for _, snapshot := range snapshots {
+                                               value, err := 
cache.get(b.Context(), snapshot, func(context.Context) (snapshotManifestSet, 
error) {
+                                                       return 
sets[snapshot.SnapshotID-1], nil
+                                               })
+                                               if err != nil {
+                                                       b.Fatal(err)
+                                               }
+                                               for _, manifest := range 
value.borrowAllManifests() {
+                                                       path := 
manifest.FilePath()
+                                                       if _, ok := seen[path]; 
ok {
+                                                               continue
+                                                       }
+                                                       seen[path] = struct{}{}
+                                                       yielded++
+                                               }
+                                       }
+                                       if yielded != count {
+                                               b.Fatal(yielded)
+                                       }
+                               }
+                       })
+                       b.Run("prefetch", func(b *testing.B) {
+                               b.ReportAllocs()
+                               for b.Loop() {
+                                       var yielded int
+                                       streamSnapshotManifests(b.Context(), 
snapshots, cache.tryGetComplete, load,
+                                               func(_ iceberg.ManifestFile, 
err error) bool {
+                                                       if err != nil {
+                                                               b.Fatal(err)
+                                                       }
+                                                       yielded++
+
+                                                       return true
+                                               })
+                                       if yielded != count {
+                                               b.Fatal(yielded)
+                                       }
+                               }
+                       })
+               })
+       }
+}
diff --git a/table/snapshot_manifest_cache.go b/table/snapshot_manifest_cache.go
index 8f8e56611..8b63f9736 100644
--- a/table/snapshot_manifest_cache.go
+++ b/table/snapshot_manifest_cache.go
@@ -218,6 +218,21 @@ func (c *snapshotManifestCache) get(
        }
 }
 
+// tryGetComplete returns a completed manifest list without starting or joining
+// a read. Inspect can use this to keep warm-cache traversal synchronous while
+// still prefetching cache misses concurrently.
+func (c *snapshotManifestCache) tryGetComplete(snapshot Snapshot) 
(snapshotManifestSet, bool) {
+       if c == nil {
+               return snapshotManifestSet{}, false
+       }
+
+       c.mu.Lock()
+       manifests, ok := c.complete.Get(snapshotManifestCacheKeyFor(snapshot))
+       c.mu.Unlock()
+
+       return manifests, ok
+}
+
 func (c *snapshotManifestCache) load(
        ctx context.Context,
        snapshot Snapshot,
@@ -352,6 +367,10 @@ func (p ManifestProvider) set(ctx context.Context, 
snapshot Snapshot) (snapshotM
        })
 }
 
+func (p ManifestProvider) cachedSet(snapshot Snapshot) (snapshotManifestSet, 
bool) {
+       return p.cache.tryGetComplete(snapshot)
+}
+
 func sharedSnapshotManifestFSF(fsF FSysF) FSysF {
        var (
                once sync.Once

Reply via email to