Remove naive compactor. (#609)

Author: ggreer

pkg/synccompactor/benchmark_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/conductorone/baton-sdk/pkg/connectorstore"
 	"github.com/conductorone/baton-sdk/pkg/dotc1z"
 	"github.com/conductorone/baton-sdk/pkg/synccompactor/attached"
-	"github.com/conductorone/baton-sdk/pkg/synccompactor/naive"
 	"github.com/stretchr/testify/require"
 )
 
@@ -209,66 +208,6 @@ func generateTestData(ctx context.Context, t *testing.B, tmpDir string, dataset
 	return baseFile, baseSyncID, appliedFile, appliedSyncID
 }
 
-// benchmarkNaiveCompactor runs a benchmark using the naive compactor.
-func benchmarkNaiveCompactor(b *testing.B, dataset BenchmarkData) {
-	ctx := context.Background()
-
-	for b.Loop() {
-		b.StopTimer()
-
-		// Create temporary directories
-		tmpDir, err := os.MkdirTemp("", "benchmark-naive")
-		require.NoError(b, err)
-		defer os.RemoveAll(tmpDir)
-
-		outputDir, err := os.MkdirTemp("", "benchmark-output")
-		require.NoError(b, err)
-		defer os.RemoveAll(outputDir)
-
-		// Generate test data
-		baseFile, _, appliedFile, _ := generateTestData(ctx, b, tmpDir, dataset)
-
-		opts := []dotc1z.C1ZOption{
-			dotc1z.WithPragma("journal_mode", "WAL"),
-		}
-
-		// Use naive compactor
-		baseC1Z, err := dotc1z.NewC1ZFile(ctx, baseFile, opts...)
-		require.NoError(b, err)
-		defer baseC1Z.Close()
-
-		appliedC1Z, err := dotc1z.NewC1ZFile(ctx, appliedFile, opts...)
-		require.NoError(b, err)
-		defer appliedC1Z.Close()
-
-		destFile := filepath.Join(tmpDir, "naive-dest.c1z")
-		destOpts := []dotc1z.C1ZOption{
-			dotc1z.WithTmpDir(tmpDir),
-		}
-		destOpts = append(destOpts, opts...)
-		destC1Z, err := dotc1z.NewC1ZFile(ctx, destFile, destOpts...)
-		require.NoError(b, err)
-		defer destC1Z.Close()
-
-		// Start a sync in the destination file
-		_, err = destC1Z.StartNewSync(ctx, connectorstore.SyncTypeFull, "")
-		require.NoError(b, err)
-
-		b.StartTimer()
-
-		// Benchmark the naive compaction
-		naiveCompactor := naive.NewNaiveCompactor(baseC1Z, appliedC1Z, destC1Z)
-		err = naiveCompactor.Compact(ctx)
-		require.NoError(b, err)
-
-		b.StopTimer()
-
-		// End the sync
-		err = destC1Z.EndSync(ctx)
-		require.NoError(b, err)
-	}
-}
-
 // benchmarkAttachedCompactor runs a benchmark using the attached compactor.
 func benchmarkAttachedCompactor(b *testing.B, dataset BenchmarkData) {
 	ctx := context.Background()
@@ -328,26 +267,14 @@ func benchmarkAttachedCompactor(b *testing.B, dataset BenchmarkData) {
 
 // Benchmark functions for different data sizes and approaches
 
-func BenchmarkNaiveCompactor_Small(b *testing.B) {
-	benchmarkNaiveCompactor(b, SmallDataset)
-}
-
 func BenchmarkAttachedCompactor_Small(b *testing.B) {
 	benchmarkAttachedCompactor(b, SmallDataset)
 }
 
-func BenchmarkNaiveCompactor_Medium(b *testing.B) {
-	benchmarkNaiveCompactor(b, MediumDataset)
-}
-
 func BenchmarkAttachedCompactor_Medium(b *testing.B) {
 	benchmarkAttachedCompactor(b, MediumDataset)
 }
 
-func BenchmarkNaiveCompactor_Large(b *testing.B) {
-	benchmarkNaiveCompactor(b, LargeDataset)
-}
-
 func BenchmarkAttachedCompactor_Large(b *testing.B) {
 	benchmarkAttachedCompactor(b, LargeDataset)
 }

pkg/synccompactor/compactor.go

@@ -16,7 +16,6 @@ import (
 	"github.com/conductorone/baton-sdk/pkg/sdk"
 	"github.com/conductorone/baton-sdk/pkg/sync"
 	"github.com/conductorone/baton-sdk/pkg/synccompactor/attached"
-	"github.com/conductorone/baton-sdk/pkg/synccompactor/naive"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
 	"go.opentelemetry.io/otel"
 	"go.uber.org/zap"
@@ -330,13 +329,6 @@ func (c *Compactor) doOneCompaction(ctx context.Context, base *CompactableSync,
 	}
 
 	switch c.compactorType {
-	case CompactorTypeNaive:
-		// TODO: Add support for syncID or remove naive compactor.
-		runner := naive.NewNaiveCompactor(baseFile, appliedFile, newFile)
-		if err := runner.Compact(ctx); err != nil {
-			l.Error("error running compaction", zap.Error(err))
-			return nil, err
-		}
 	case CompactorTypeAttached:
 		runner := attached.NewAttachedCompactor(baseFile, appliedFile, newFile)
 		if err := runner.CompactWithSyncID(ctx, newSyncId); err != nil {

pkg/synccompactor/compactor_test.go

@@ -36,27 +36,6 @@ func TestAttachedCompactorWithTmpDir(t *testing.T) {
 	})
 }
 
-func TestNaiveCompactorWithTmpDir(t *testing.T) {
-	ctx := context.Background()
-
-	inputSyncsDir, err := os.MkdirTemp("", "compactor-test")
-	require.NoError(t, err)
-	defer os.RemoveAll(inputSyncsDir)
-
-	outputDir, err := os.MkdirTemp("", "compactor-output")
-	require.NoError(t, err)
-	defer os.RemoveAll(outputDir)
-
-	// Create temporary directory for intermediate files
-	tmpDir, err := os.MkdirTemp("", "compactor-tmp")
-	require.NoError(t, err)
-	defer os.RemoveAll(tmpDir)
-
-	runCompactorTest(t, ctx, inputSyncsDir, func(compactableSyncs []*CompactableSync) (*Compactor, func() error, error) {
-		return NewCompactor(ctx, outputDir, compactableSyncs, WithTmpDir(tmpDir), WithCompactorType(CompactorTypeNaive))
-	})
-}
-
 func runCompactorTest(t *testing.T, ctx context.Context, inputSyncsDir string, createCompactor func([]*CompactableSync) (*Compactor, func() error, error)) {
 	opts := []dotc1z.C1ZOption{
 		dotc1z.WithPragma("journal_mode", "WAL"),
@@ -732,10 +711,6 @@ func TestSyncTypeUnion_AttachedCompactor(t *testing.T) {
 	runSyncTypeUnionTests(t, CompactorTypeAttached, getAllSyncTypeTestCases())
 }
 
-func TestSyncTypeUnion_NaiveCompactor(t *testing.T) {
-	runSyncTypeUnionTests(t, CompactorTypeNaive, getBasicSyncTypeTestCases())
-}
-
 // getAllSyncTypeTestCases returns comprehensive test cases for sync type union logic.
 func getAllSyncTypeTestCases() []syncTypeTestCase {
 	return []syncTypeTestCase{
@@ -817,32 +792,6 @@ func getAllSyncTypeTestCases() []syncTypeTestCase {
 	}
 }
 
-// getBasicSyncTypeTestCases returns a subset of test cases for basic validation.
-func getBasicSyncTypeTestCases() []syncTypeTestCase {
-	return []syncTypeTestCase{
-		{
-			name:     "Full + Partial = Full",
-			input:    []connectorstore.SyncType{connectorstore.SyncTypeFull, connectorstore.SyncTypePartial},
-			expected: connectorstore.SyncTypeFull,
-		},
-		{
-			name:     "ResourcesOnly + Partial = ResourcesOnly",
-			input:    []connectorstore.SyncType{connectorstore.SyncTypeResourcesOnly, connectorstore.SyncTypePartial},
-			expected: connectorstore.SyncTypeResourcesOnly,
-		},
-		{
-			name:     "Partial + Partial = Partial",
-			input:    []connectorstore.SyncType{connectorstore.SyncTypePartial, connectorstore.SyncTypePartial},
-			expected: connectorstore.SyncTypePartial,
-		},
-		{
-			name:     "Full + ResourcesOnly + Partial = Full",
-			input:    []connectorstore.SyncType{connectorstore.SyncTypeFull, connectorstore.SyncTypeResourcesOnly, connectorstore.SyncTypePartial},
-			expected: connectorstore.SyncTypeFull,
-		},
-	}
-}
-
 type syncTypeTestCase struct {
 	name     string
 	input    []connectorstore.SyncType