From 445973eaf8dea5cd7abd47367846f754362d48ea Mon Sep 17 00:00:00 2001 From: Neil Macneale IV Date: Tue, 26 Aug 2025 11:31:27 -0700 Subject: [PATCH] PR Feedback --- go/store/nbs/archive_writer.go | 9 ++------- go/store/nbs/store.go | 15 +-------------- .../gc_oldgen_conjoin_test.go | 2 +- 3 files changed, 4 insertions(+), 22 deletions(-) diff --git a/go/store/nbs/archive_writer.go b/go/store/nbs/archive_writer.go index c502fa0b67..c3d02b4482 100644 --- a/go/store/nbs/archive_writer.go +++ b/go/store/nbs/archive_writer.go @@ -795,7 +795,7 @@ func planArchiveConjoin(sources []sourceWithSize, stats *Stats) (compactionPlan, // place largest chunk sources at the beginning of the conjoin orderedSrcs := chunkSourcesByDescendingDataSize{sws: sources} - // sort.Sort(orderedSrcs) + sort.Sort(orderedSrcs) sources = nil writer := NewBlockBufferByteSink(fourMb) @@ -809,12 +809,7 @@ func planArchiveConjoin(sources []sourceWithSize, stats *Stats) (compactionPlan, arcSrc, ok := reader.(archiveChunkSource) if !ok { // When it's not an archive, we want to use the table index to extract chunk records one at a time. - tblSrc, ok := reader.(chunkSource) - if !ok { - return compactionPlan{}, fmt.Errorf("runtime error: source %T is not an archiveChunkSource or fileTableReader", reader) - } - - index, err := tblSrc.index() + index, err := reader.index() if err != nil { return compactionPlan{}, err } diff --git a/go/store/nbs/store.go b/go/store/nbs/store.go index 69ad768e8e..ac061dd4cd 100644 --- a/go/store/nbs/store.go +++ b/go/store/nbs/store.go @@ -2386,7 +2386,7 @@ func (nbs *NomsBlockStore) ConjoinTableFiles(ctx context.Context, storageIds []h } // findTableSpec finds a table spec by hash in the current manifest. This will ignore novel tables, as it's not -// needed currently. This function also checks for archive files via the persister. +// needed currently. func (nbs *NomsBlockStore) findTableSpec(storageId hash.Hash) (tableSpec, bool) { for _, spec := range nbs.upstream.specs { if spec.name == storageId { @@ -2402,18 +2402,5 @@ func (nbs *NomsBlockStore) findTableSpec(storageId hash.Hash) (tableSpec, bool) return tableSpec{name: storageId, chunkCount: count}, true } } - - // Check if the storage ID corresponds to an archive file by attempting to open it - // The persister's Open method will check for both table files and archive files - ctx := context.Background() - cs, err := nbs.persister.Open(ctx, storageId, 0, nbs.stats) - if err == nil { - defer cs.close() - count, err := cs.count() - if err == nil { - return tableSpec{name: storageId, chunkCount: count}, true - } - } - return tableSpec{name: storageId, chunkCount: 0}, false } diff --git a/integration-tests/go-sql-server-driver/gc_oldgen_conjoin_test.go b/integration-tests/go-sql-server-driver/gc_oldgen_conjoin_test.go index fc8b14f0f3..73816faa76 100644 --- a/integration-tests/go-sql-server-driver/gc_oldgen_conjoin_test.go +++ b/integration-tests/go-sql-server-driver/gc_oldgen_conjoin_test.go @@ -50,7 +50,7 @@ func TestGCConjoinsOldgen(t *testing.T) { for _, tc := range cases { tc := tc t.Run(tc.name, func(t *testing.T) { - // NOTE: avoid t.Parallel() here to keep dynamic ports simple. + t.Parallel() var ports DynamicResources ports.global = &GlobalPorts ports.t = t