Skip to content

Commit 537c161

Browse files
committed
vecindex: delete Manager.GetWithDesc() method
This method was added because backfilling indexes are not considered deletable. In retrospect, there's no reason why we need to restrict an internal interface this way and deleting it removes a fair bit of complexity from upcoming changes to the vector store. Informs: #146046 Release note: None
1 parent 03c6832 commit 537c161

File tree

4 files changed

+24
-96
lines changed

4 files changed

+24
-96
lines changed

pkg/sql/backfill/backfill.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ func (ib *IndexBackfiller) initIndexes(
891891
return err
892892
}
893893

894-
vecIndex, err := vecIndexManager.GetWithDesc(ctx, desc, idx)
894+
vecIndex, err := vecIndexManager.Get(ctx, desc.GetID(), idx.GetID())
895895
if err != nil {
896896
return err
897897
}

pkg/sql/vecindex/manager.go

Lines changed: 22 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,11 @@ func (m *Manager) Metrics() metric.Struct {
8989
return &m.metrics
9090
}
9191

92-
// getImpl returns the vector index for the given DB table and index. If the DB
92+
// Get returns the vector index for the given DB table and index. If the DB
9393
// index does not currently have an active vector index, one is created and
9494
// cached.
95-
func (m *Manager) getImpl(
96-
ctx context.Context,
97-
tableID catid.DescID,
98-
indexID catid.IndexID,
99-
makeIndex func() (*cspann.Index, error),
95+
func (m *Manager) Get(
96+
ctx context.Context, tableID catid.DescID, indexID catid.IndexID,
10097
) (*cspann.Index, error) {
10198
m.mu.Lock()
10299
defer m.mu.Unlock()
@@ -133,8 +130,24 @@ func (m *Manager) getImpl(
133130
if m.testingKnobs != nil && m.testingKnobs.DuringVecIndexPull != nil {
134131
m.testingKnobs.DuringVecIndexPull()
135132
}
136-
137-
idx, err := makeIndex()
133+
config, err := m.getVecConfig(ctx, tableID, indexID)
134+
if err != nil {
135+
return nil, err
136+
}
137+
// TODO(drewk): use the config to populate the index options as well.
138+
quantizer := quantize.NewRaBitQuantizer(int(config.Dims), config.Seed, config.DistanceMetric)
139+
store, err := vecstore.New(ctx, m.db, quantizer, m.codec, tableID, indexID)
140+
if err != nil {
141+
return nil, err
142+
}
143+
// Use the stored context so that the vector index can outlive the context
144+
// of the Get call. The fixup process gets a child context from the context
145+
// passed to cspann.NewIndex, and we don't want that to be the context of
146+
// the Get call.
147+
idx, err := cspann.NewIndex(
148+
m.ctx, store, quantizer, config.Seed,
149+
m.getIndexOptions(&config, store.ReadOnly()), m.stopper,
150+
)
138151
if err != nil {
139152
return nil, err
140153
}
@@ -158,68 +171,6 @@ func (m *Manager) getImpl(
158171
return idx, err
159172
}
160173

161-
// GetWithDesc returns a cached cspann vector index for a given table and index
162-
// using the provided table descriptor and index.
163-
func (m *Manager) GetWithDesc(
164-
ctx context.Context, desc catalog.TableDescriptor, index catalog.Index,
165-
) (*cspann.Index, error) {
166-
return m.getImpl(
167-
ctx,
168-
desc.GetID(),
169-
index.GetID(),
170-
func() (*cspann.Index, error) {
171-
// TODO(drewk): use the config to populate the index options as well.
172-
config := index.GetVecConfig()
173-
quantizer := quantize.NewRaBitQuantizer(
174-
int(config.Dims), config.Seed, config.DistanceMetric)
175-
store, err := vecstore.NewWithColumnID(
176-
ctx, m.db, quantizer, m.codec, desc, index.GetID(), index.VectorColumnID(),
177-
)
178-
if err != nil {
179-
return nil, err
180-
}
181-
182-
return cspann.NewIndex(
183-
m.ctx, store, quantizer, config.Seed,
184-
m.getIndexOptions(&config, store.ReadOnly()), m.stopper,
185-
)
186-
},
187-
)
188-
}
189-
190-
// Get returns a cached cspann vector index for a given table and index using the
191-
// descriptor IDs for both.
192-
func (m *Manager) Get(
193-
ctx context.Context, tableID catid.DescID, indexID catid.IndexID,
194-
) (*cspann.Index, error) {
195-
return m.getImpl(
196-
ctx,
197-
tableID,
198-
indexID,
199-
func() (*cspann.Index, error) {
200-
config, err := m.getVecConfig(ctx, tableID, indexID)
201-
if err != nil {
202-
return nil, err
203-
}
204-
// TODO(drewk): use the config to populate the index options as well.
205-
quantizer := quantize.NewRaBitQuantizer(
206-
int(config.Dims), config.Seed, config.DistanceMetric)
207-
store, err := vecstore.New(ctx, m.db, quantizer, m.codec, tableID, indexID)
208-
if err != nil {
209-
return nil, err
210-
}
211-
// Use the stored context so that the vector index can outlive the context
212-
// of the Get call. The fixup process gets a child context from the context
213-
// passed to cspann.NewIndex, and we don't want that to be the context of
214-
// the Get call.
215-
return cspann.NewIndex(
216-
m.ctx, store, quantizer, config.Seed,
217-
m.getIndexOptions(&config, store.ReadOnly()), m.stopper,
218-
)
219-
},
220-
)
221-
}
222-
223174
func (m *Manager) getIndexOptions(config *vecpb.Config, readOnly bool) *cspann.IndexOptions {
224175
return &cspann.IndexOptions{
225176
RotAlgorithm: config.RotAlgorithm,
@@ -256,7 +207,7 @@ func (m *Manager) getVecConfig(
256207
return vecpb.Config{}, errTableNotFound
257208
}
258209
var idxDesc catalog.Index
259-
for _, desc := range tableDesc.DeletableNonPrimaryIndexes() {
210+
for _, desc := range tableDesc.NonPrimaryIndexes() {
260211
if desc.GetID() == indexID {
261212
idxDesc = desc
262213
break

pkg/sql/vecindex/manager_test.go

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -188,29 +188,6 @@ func TestVectorManager(t *testing.T) {
188188
require.Error(t, err)
189189
})
190190

191-
t.Run("test GetWithDesc functionality", func(t *testing.T) {
192-
var tableDesc catalog.TableDescriptor
193-
err := internalDB.DescsTxn(ctx, func(ctx context.Context, txn descs.Txn) error {
194-
var err error
195-
tableDesc, err = txn.Descriptors().ByIDWithLeased(txn.KV()).Get().Table(ctx, 141)
196-
return err
197-
})
198-
require.NoError(t, err)
199-
var idxDesc catalog.Index
200-
for _, desc := range tableDesc.DeletableNonPrimaryIndexes() {
201-
if desc.GetID() == 2 {
202-
idxDesc = desc
203-
break
204-
}
205-
}
206-
// Pull an index using descriptors.
207-
_, err = vectorMgr.GetWithDesc(ctx, tableDesc, idxDesc)
208-
require.NoError(t, err)
209-
// Pull the index again.
210-
_, err = vectorMgr.GetWithDesc(ctx, tableDesc, idxDesc)
211-
require.NoError(t, err)
212-
})
213-
214191
t.Run("test multiple threaded functionality", func(t *testing.T) {
215192
pullDelayer := sync.WaitGroup{}
216193
pullDelayer.Add(10)

pkg/sql/vecindex/vecstore/store.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func New(
125125
}
126126

127127
var index catalog.Index
128-
for _, desc := range tableDesc.DeletableNonPrimaryIndexes() {
128+
for _, desc := range tableDesc.NonPrimaryIndexes() {
129129
if desc.GetID() == indexID {
130130
index = desc
131131
break

0 commit comments

Comments
 (0)