fix #58 - allow to download the shard in parallel#61
Open
TaranpreetSingh wants to merge 1 commit intothanos-io:mainfrom
Open
fix #58 - allow to download the shard in parallel#61TaranpreetSingh wants to merge 1 commit intothanos-io:mainfrom
TaranpreetSingh wants to merge 1 commit intothanos-io:mainfrom
Conversation
Signed-off-by: Taranpreet Singh <tsingh@roku.com>
TaranpreetSingh
changed the title
TaranpreetSingh
changed the title
GiedriusS
reviewed
Feb 17, 2026
| go func(shardIndex int) { | ||
| defer wg.Done() | ||
|
|
||
| sem <- struct{}{} |
Member
There was a problem hiding this comment.
This is too complicated, I think. Instead we typically spawn shardConcurrency goroutines that read the shardIndex from a channel in a for range loop. Then, you don't need to do this complex logic with sem.
GiedriusS
reviewed
Feb 17, 2026
| sem := make(chan struct{}, shardConcurrency) | ||
|
|
||
| for i := range int(m.Shards) { | ||
| wg.Add(1) |
Member
There was a problem hiding this comment.
You can modernize with wg.Go().
GiedriusS
reviewed
Feb 17, 2026
| var shards []*db.Shard | ||
| var err error | ||
|
|
||
| // Use parallel loading if shardConcurrency > 0, otherwise use sequential (original) loading |
Member
There was a problem hiding this comment.
I hope that everyone working on this repo is able to read a simple if statement (:
Author
There was a problem hiding this comment.
this was for my readability will remove it
GiedriusS
reviewed
Feb 17, 2026
| shards = append(shards, result.shard) | ||
| } | ||
|
|
||
| // Return error only if ALL shards failed |
Member
There was a problem hiding this comment.
I can see that. This comment has zero value.
Author
There was a problem hiding this comment.
this was for my readability will remove it
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change introduces configurable parallel shard loading to address performance bottlenecks during block synchronization. Previously, the service loaded shards sequentially - waiting for each shard to complete before starting the next one. This approach underutilized available network bandwidth and caused timeout errors when loading large blocks, particularly during service startup.
The implementation adds a new CLI flag
--block.syncer.shard-concurrencythat allows multiple shards to be loaded simultaneously. When set to a value greater than zero, the syncer uses a newreadShardsParallel()function that spawns concurrent goroutines controlled by a semaphore pattern. This keeps network connections busy and dramatically reduces sync times. The original sequential loading behavior is preserved when the flag is set to zero (the default), ensuring complete backward compatibility.The change required minimal modifications: adding the CLI flag in
serve.go, wiring it through the configuration inconfig.go, and implementing the parallel loading logic alongside the existing sequential path insyncer.go. The originalreadShards()function remains unchanged, with the newreadShardsParallel()function handling concurrent operations. A simple conditional innewBlockForMeta()chooses between the two approaches based on the configured concurrency value.This feature is particularly beneficial for services running on high-bandwidth infrastructure where network capacity significantly exceeds what sequential loading can utilize. By enabling parallel shard loading, administrators can achieve 50-65% faster sync times, reduce timeout errors, and improve overall service availability during startup and periodic syncs.