Skip to content

Commit

Permalink
fix: Fix duplicate enqueue item problem in bloom download queue when …
Browse files Browse the repository at this point in the history
…do sync download (#13114)

A missing return will cause item enqueued twice in sync download scenario. That cause the response not like expected.

Suppose we want to fetch a blocks array: [0, 1, 2]. Enqueued items are [0, 0, 1, 1, 2, 2],If we want response sorted we wait 3 times response will get [0, 0, 1]
  • Loading branch information
honganan authored Jun 10, 2024
1 parent 2b19dac commit f98ff7f
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/storage/stores/shipper/bloomshipper/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ func newDownloadQueue[T any, R any](size, workers int, process processFunc[T, R]
func (q *downloadQueue[T, R]) enqueue(t downloadRequest[T, R]) {
if !t.async {
q.queue <- t
return
}
// for async task we attempt to dedupe task already in progress.
q.enqueuedMutex.Lock()
Expand Down
53 changes: 53 additions & 0 deletions pkg/storage/stores/shipper/bloomshipper/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,59 @@ func TestFetcher_DownloadQueue(t *testing.T) {
}

})

t.Run("download multiple items and return in order", func(t *testing.T) {
ctx := context.Background()

q, err := newDownloadQueue[bool, bool](
100,
1,
func(_ context.Context, r downloadRequest[bool, bool]) {
r.results <- downloadResponse[bool]{
key: r.key,
idx: r.idx,
item: true,
}
},
log.NewNopLogger(),
)
require.NoError(t, err)

count := 10
resultsCh := make(chan downloadResponse[bool], count)
errorsCh := make(chan error, count)

reqs := buildDownloadRequest(ctx, count, resultsCh, errorsCh)
for _, r := range reqs {
q.enqueue(r)
}

for i := 0; i < count; i++ {
select {
case err := <-errorsCh:
require.False(t, true, "got %+v should have received a response instead", err)
case res := <-resultsCh:
require.True(t, res.item)
require.Equal(t, reqs[i].key, res.key)
require.Equal(t, reqs[i].idx, res.idx)
}
}
})
}

func buildDownloadRequest(ctx context.Context, count int, resCh chan downloadResponse[bool], errCh chan error) []downloadRequest[bool, bool] {
requests := make([]downloadRequest[bool, bool], count)
for i := 0; i < count; i++ {
requests[i] = downloadRequest[bool, bool]{
ctx: ctx,
item: false,
key: "test",
idx: i,
results: resCh,
errors: errCh,
}
}
return requests
}

func TestFetcher_LoadBlocksFromFS(t *testing.T) {
Expand Down

0 comments on commit f98ff7f

Please sign in to comment.