Skip to content

Commit 4fd11e0

Browse files
committed
fix(pullsync): cap Topmost at contiguous frontier to prevent mid-offer gap advancement
collectAddrs tracks the highest BinID reachable from start without a gap (contiguousEnd). When the server's store has internal gaps (e.g. BinIDs {3,7,11} with start=3), all matching chunks are still included in the offer for eager client-side storage, but Topmost is capped at contiguousEnd (3) so the client's sync interval does not advance past missing BinIDs. The cap only fires when contiguousEnd >= start (at least one historical chunk extended the frontier). This avoids capping when the offer contains only live chunks (BinID > historicalCursor), where topmost is already bounded by the existing historicalCursor cap. Adds TestSync_MidOfferGapCapsTopmostAtContiguousFrontier to cover this case.
1 parent e0c56cc commit 4fd11e0

2 files changed

Lines changed: 70 additions & 4 deletions

File tree

pkg/pullsync/pullsync.go

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,11 +461,15 @@ func (s *Syncer) collectAddrs(ctx context.Context, bin uint8, start uint64) ([]*
461461
}
462462

463463
var (
464-
chs []*storer.BinC
465-
topmost uint64
466-
timer *time.Timer
467-
timerC <-chan time.Time
464+
chs []*storer.BinC
465+
topmost uint64
466+
timer *time.Timer
467+
timerC <-chan time.Time
468+
contiguousEnd uint64 // highest BinID contiguous from start; only meaningful when start > 0
468469
)
470+
if start > 0 {
471+
contiguousEnd = start - 1
472+
}
469473
chC, unsub, errC := s.store.SubscribeBin(ctx, bin, start)
470474
defer func() {
471475
unsub()
@@ -495,6 +499,13 @@ func (s *Syncer) collectAddrs(ctx context.Context, bin uint8, start uint64) ([]*
495499
break LOOP
496500
}
497501

502+
// Track the contiguous frontier: the highest BinID reachable
503+
// from start without a gap. Used after the loop to cap Topmost
504+
// so the client's interval does not advance past missing BinIDs.
505+
if c.BinID == contiguousEnd+1 {
506+
contiguousEnd = c.BinID
507+
}
508+
498509
chs = append(chs, &storer.BinC{Address: c.Address, BatchID: c.BatchID, StampHash: c.StampHash})
499510
if c.BinID > topmost {
500511
topmost = c.BinID
@@ -527,6 +538,16 @@ func (s *Syncer) collectAddrs(ctx context.Context, bin uint8, start uint64) ([]*
527538
topmost = historicalCursor
528539
}
529540

541+
// Cap topmost at the contiguous frontier. All collected chunks are
542+
// included in the offer for eager client-side storage, but Topmost is
543+
// bounded to the highest BinID reachable from start without a gap.
544+
// The client stores every chunk in one round trip; subsequent round
545+
// trips advance the interval bookkeeping via leading-gap empty offers
546+
// and empty Want bitvectors — no chunk data is retransmitted.
547+
if start > 0 && len(chs) > 0 && contiguousEnd >= start && contiguousEnd < topmost {
548+
topmost = contiguousEnd
549+
}
550+
530551
return &collectAddrsResult{chs: chs, topmost: topmost}, nil
531552
})
532553
if err != nil {

pkg/pullsync/pullsync_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,51 @@ func TestSync_HistoricalGapReturnsEmptyOfferAtBoundary(t *testing.T) {
440440
})
441441
}
442442

443+
// TestSync_MidOfferGapCapsTopmostAtContiguousFrontier verifies that when the
444+
// server's offer contains an internal gap (chunks at BinIDs {3,7,11} with
445+
// start=3), Topmost is capped at the contiguous frontier (3) so the client's
446+
// interval does not advance past the gap. All chunks are still included in
447+
// the offer for eager storage — no chunk data is retransmitted on later rounds.
448+
func TestSync_MidOfferGapCapsTopmostAtContiguousFrontier(t *testing.T) {
449+
synctest.Test(t, func(t *testing.T) {
450+
ch1 := testingc.GenerateTestRandomChunk()
451+
ch2 := testingc.GenerateTestRandomChunk()
452+
ch3 := testingc.GenerateTestRandomChunk()
453+
454+
makeResult := func(c swarm.Chunk, binID uint64) *storer.BinC {
455+
h, err := c.Stamp().Hash()
456+
if err != nil {
457+
t.Fatal(err)
458+
}
459+
return &storer.BinC{Address: c.Address(), BatchID: c.Stamp().BatchID(), BinID: binID, StampHash: h}
460+
}
461+
462+
// BinIDs: 3, 7, 11 — gaps at [4,6] and [8,10].
463+
results := []*storer.BinC{makeResult(ch1, 3), makeResult(ch2, 7), makeResult(ch3, 11)}
464+
const cursor = uint64(11)
465+
466+
var (
467+
ps, _ = newPullSync(t, nil, 10, mock.WithSubscribeResp(results, nil), mock.WithChunks(ch1, ch2, ch3), mock.WithCursors([]uint64{cursor}, 0))
468+
recorder = streamtest.New(streamtest.WithProtocols(ps.Protocol()))
469+
psClient, db = newPullSync(t, recorder, 0)
470+
)
471+
472+
topmost, count, err := psClient.Sync(context.Background(), swarm.ZeroAddress, 0, 3)
473+
if err != nil {
474+
t.Fatal(err)
475+
}
476+
// Topmost must be the contiguous frontier (3), not the max BinID (11).
477+
if topmost != 3 {
478+
t.Fatalf("topmost: got %d, want 3 (contiguous frontier)", topmost)
479+
}
480+
// All three chunks must be delivered and stored in this single round trip.
481+
if count != 3 {
482+
t.Fatalf("count: got %d, want 3 (all chunks delivered eagerly)", count)
483+
}
484+
haveChunks(t, db, ch1, ch2, ch3)
485+
})
486+
}
487+
443488
// TestSync_OverwriteNewerChunkDoesNotBlockInterval verifies that
444489
// ErrOverwriteNewerChunk does not prevent interval advancement.
445490
// The chunk is already present in the reserve with a newer stamp, so it is

0 commit comments

Comments
 (0)