merging: In compound shards, sort repos by id #887
Conversation
We sort shards in compound shards by repository ID so that we can use binary search to quickly find a specific repo when calling `build.findShard`. We have seen that for very large compound shards it can take a long time to find a specific repo which can slow down initial indexing even for no-op indexes after a restart of indexserver. The sort order will impact ranking of non-bm25 search, because doc-order acts as a tie-breaker. However, the actual impact is likely very small, because "LastCommitDate" is a stronger signal. Test plan: New unit test
| sort.Slice(ds, func(i, j int) bool { | ||
| return ds[i].repoMetaData[0].priority > ds[j].repoMetaData[0].priority | ||
| return ds[i].repoMetaData[0].ID < ds[j].repoMetaData[0].ID |
There was a problem hiding this comment.
It'd be good to document this invariant on indexData.repoMetaData (that this list must always be sorted by repo ID).
I first wanted to find out if there are any self-hosted customers who have enabled shard merging. If not, we don't have to worry about falling back to brute force search if binary search doesn't return a result. |
I thought a bit more about this. We have to assume that all self-hosted customers have shard merging enabled. I think the best course of action is to increment the indexFormatVersion and use that to signal when we can use binary search. I will update this PR once I have something ready. |
We sort shards in compound shards by repository ID so that we can use binary search instead of looping through the entire list of repos when when calling
build.findShard.We have seen that for very large compound shards (5k+ repos) it can take a long time to find a specific repo which can slow down initial indexing even for no-op indexes after a restart of indexserver.
The sort order will impact ranking of non-bm25 search, because doc-order acts as a tie-breaker. However, the actual impact is very small, because "LastCommitDate" is a stronger signal.
Note:
We moved from
prioritytoLatestCommitDateas tie breaker in #832. Hence, sorting repos in compound shards by priority doesn't make much sense anymore.Test plan:
New unit test