From 433357e0c5ab80e208901c87df2db23d7cef9d1f Mon Sep 17 00:00:00 2001 From: Keegan Carruthers-Smith Date: Mon, 19 May 2025 15:22:22 +0200 Subject: [PATCH] index: remove Options.{TenantID,RepoID} These are duplicated with RepositoryDescription. There is exactly one place we set these values, and in that case we set it in the repository description as well. Note: we update some the calls in builder.go for o.IndexOptions.RepoID to be just o.RepoID. This is to make it consistent with how we access TenantID. IndexOptions is embedded in that struct (indexArgs) which is why we can do the shortcut. Test Plan: go test is sufficient for this change. I will do a larger round of manual tests soon with all my changes stacked. --- cmd/zoekt-sourcegraph-indexserver/index.go | 7 ++----- index/builder.go | 10 ++-------- index/merge.go | 5 ----- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/cmd/zoekt-sourcegraph-indexserver/index.go b/cmd/zoekt-sourcegraph-indexserver/index.go index 3d941e81d..9d082a0ef 100644 --- a/cmd/zoekt-sourcegraph-indexserver/index.go +++ b/cmd/zoekt-sourcegraph-indexserver/index.go @@ -110,11 +110,11 @@ func (o *indexArgs) BuildOptions() *index.Options { // nothing needs to be done. RepositoryDescription: zoekt.Repository{ TenantID: o.TenantID, - ID: o.IndexOptions.RepoID, + ID: o.RepoID, Name: o.Name, Branches: o.Branches, RawConfig: map[string]string{ - "repoid": strconv.Itoa(int(o.IndexOptions.RepoID)), + "repoid": strconv.Itoa(int(o.RepoID)), "priority": strconv.FormatFloat(o.Priority, 'g', -1, 64), "public": marshalBool(o.Public), "fork": marshalBool(o.Fork), @@ -135,9 +135,6 @@ func (o *indexArgs) BuildOptions() *index.Options { LanguageMap: o.LanguageMap, ShardMerging: o.ShardMerging, - - TenantID: o.TenantID, - RepoID: o.RepoID, } } diff --git a/index/builder.go b/index/builder.go index ac867db8e..d02f531e9 100644 --- a/index/builder.go +++ b/index/builder.go @@ -120,12 +120,6 @@ type Options struct { // // Note: heap checking is "best effort", and it's possible for the process to OOM without triggering the heap profile. HeapProfileTriggerBytes uint64 - - // TenantID is the ID of the tenant this shard belongs to. - TenantID int - - // RepoID is the ID of the repository this shard belongs to. - RepoID uint32 } // HashOptions contains only the options in Options that upon modification leads to IndexState of IndexStateMismatch during the next index building. @@ -343,8 +337,8 @@ func (o *Options) shardNameVersion(version, n int) string { var prefix string // If tenant enforcement is enabled and we have tenant/repo IDs, use those to generate the prefix - if o.TenantID != 0 && o.RepoID != 0 && tenant.EnforceTenant() { - prefix = tenant.SrcPrefix(o.TenantID, o.RepoID) + if o.RepositoryDescription.TenantID != 0 && o.RepositoryDescription.ID != 0 && tenant.EnforceTenant() { + prefix = tenant.SrcPrefix(o.RepositoryDescription.TenantID, o.RepositoryDescription.ID) } else { prefix = o.RepositoryDescription.Name } diff --git a/index/merge.go b/index/merge.go index 7b5ff7bcb..2603032d5 100644 --- a/index/merge.go +++ b/index/merge.go @@ -211,11 +211,6 @@ func explode(dstDir string, f IndexFile, ibFuncs ...shardBuilderFunc) (map[strin opts := Options{ IndexDir: dstDir, RepositoryDescription: ib.repoList[0], - - // TODO we should remove these fields from Options and just rely on them - // being set by RepositoryDescription. - TenantID: ib.repoList[0].TenantID, - RepoID: ib.repoList[0].ID, } shardName := opts.shardNameVersion(ib.indexFormatVersion, 0)