From 1e56f5156a2a143023fa5079e63f4160e8dc656b Mon Sep 17 00:00:00 2001
From: Vaibhav Kamra <vkamra@alcion.ai>
Date: Tue, 7 Mar 2023 10:10:53 -0800
Subject: [PATCH] Only set repo configuration on init (#2717)

Setting the repository policy calls `policy.SetPolicy` in Kopia which involves repository scan. This is an
expensive operation if the repository has a lot of snapshots.

This commit changes Corso behavior to only call this on repository init and not on every connect.

#### Does this PR need a docs update or release note?

- [ ] :white_check_mark: Yes, it's included
- [x] :clock1: Yes, but in a later PR
- [ ] :no_entry: No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Test
- [ ] :computer: CI/Deployment
- [ ] :broom: Tech Debt/Cleanup

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] :muscle: Manual
- [x] :zap: Unit test
- [ ] :green_heart: E2E
---
 src/internal/kopia/conn.go      | 15 ++++++----
 src/internal/kopia/conn_test.go | 49 +++++++++++++++++++++++++--------
 2 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/src/internal/kopia/conn.go b/src/internal/kopia/conn.go
index 5a775cb4d..f9bee1e04 100644
--- a/src/internal/kopia/conn.go
+++ b/src/internal/kopia/conn.go
@@ -90,13 +90,22 @@ func (w *conn) Initialize(ctx context.Context) error {
 		return clues.Wrap(err, "initialzing repo").WithClues(ctx)
 	}
 
-	return w.commonConnect(
+	err = w.commonConnect(
 		ctx,
 		cfg.KopiaCfgDir,
 		bst,
 		cfg.CorsoPassphrase,
 		defaultCompressor,
 	)
+	if err != nil {
+		return err
+	}
+
+	if err := w.setDefaultConfigValues(ctx); err != nil {
+		return clues.Stack(err).WithClues(ctx)
+	}
+
+	return nil
 }
 
 func (w *conn) Connect(ctx context.Context) error {
@@ -154,10 +163,6 @@ func (w *conn) commonConnect(
 		return clues.Stack(err).WithClues(ctx)
 	}
 
-	if err := w.setDefaultConfigValues(ctx); err != nil {
-		return clues.Stack(err).WithClues(ctx)
-	}
-
 	return nil
 }
 
diff --git a/src/internal/kopia/conn_test.go b/src/internal/kopia/conn_test.go
index 11145705b..2821cedd9 100644
--- a/src/internal/kopia/conn_test.go
+++ b/src/internal/kopia/conn_test.go
@@ -238,26 +238,36 @@ func (suite *WrapperIntegrationSuite) TestSetCompressor() {
 	)
 }
 
-func (suite *WrapperIntegrationSuite) TestConfigDefaultsSetOnInitAndConnect() {
+func (suite *WrapperIntegrationSuite) TestConfigDefaultsSetOnInitAndNotOnConnect() {
+	newCompressor := "pgzip"
+	newRetentionDaily := policy.OptionalInt(42)
+	newRetention := policy.RetentionPolicy{KeepDaily: &newRetentionDaily}
+	newSchedInterval := time.Second * 42
+
 	table := []struct {
-		name      string
-		checkFunc func(*testing.T, *policy.Policy)
-		mutator   func(context.Context, *policy.Policy) error
+		name          string
+		checkInitFunc func(*testing.T, *policy.Policy)
+		checkFunc     func(*testing.T, *policy.Policy)
+		mutator       func(context.Context, *policy.Policy) error
 	}{
 		{
 			name: "Compression",
-			checkFunc: func(t *testing.T, p *policy.Policy) {
+			checkInitFunc: func(t *testing.T, p *policy.Policy) {
 				t.Helper()
 				require.Equal(t, defaultCompressor, string(p.CompressionPolicy.CompressorName))
 			},
+			checkFunc: func(t *testing.T, p *policy.Policy) {
+				t.Helper()
+				require.Equal(t, newCompressor, string(p.CompressionPolicy.CompressorName))
+			},
 			mutator: func(innerCtx context.Context, p *policy.Policy) error {
-				_, res := updateCompressionOnPolicy("pgzip", p)
+				_, res := updateCompressionOnPolicy(newCompressor, p)
 				return res
 			},
 		},
 		{
 			name: "Retention",
-			checkFunc: func(t *testing.T, p *policy.Policy) {
+			checkInitFunc: func(t *testing.T, p *policy.Policy) {
 				t.Helper()
 				require.Equal(
 					t,
@@ -270,9 +280,20 @@ func (suite *WrapperIntegrationSuite) TestConfigDefaultsSetOnInitAndConnect() {
 					p.RetentionPolicy.EffectiveKeepLatest().OrDefault(42),
 				)
 			},
+			checkFunc: func(t *testing.T, p *policy.Policy) {
+				t.Helper()
+				require.Equal(
+					t,
+					newRetention,
+					p.RetentionPolicy,
+				)
+				assert.Equal(
+					t,
+					42,
+					p.RetentionPolicy.EffectiveKeepLatest().OrDefault(42),
+				)
+			},
 			mutator: func(innerCtx context.Context, p *policy.Policy) error {
-				newRetentionDaily := policy.OptionalInt(42)
-				newRetention := policy.RetentionPolicy{KeepDaily: &newRetentionDaily}
 				updateRetentionOnPolicy(newRetention, p)
 
 				return nil
@@ -280,12 +301,16 @@ func (suite *WrapperIntegrationSuite) TestConfigDefaultsSetOnInitAndConnect() {
 		},
 		{
 			name: "Scheduling",
+			checkInitFunc: func(t *testing.T, p *policy.Policy) {
+				t.Helper()
+				require.Equal(t, defaultSchedulingInterval, p.SchedulingPolicy.Interval())
+			},
 			checkFunc: func(t *testing.T, p *policy.Policy) {
 				t.Helper()
-				require.Equal(t, time.Second*0, p.SchedulingPolicy.Interval())
+				require.Equal(t, newSchedInterval, p.SchedulingPolicy.Interval())
 			},
 			mutator: func(innerCtx context.Context, p *policy.Policy) error {
-				updateSchedulingOnPolicy(time.Second*42, p)
+				updateSchedulingOnPolicy(newSchedInterval, p)
 
 				return nil
 			},
@@ -305,7 +330,7 @@ func (suite *WrapperIntegrationSuite) TestConfigDefaultsSetOnInitAndConnect() {
 			p, err := k.getPolicyOrEmpty(ctx, policy.GlobalPolicySourceInfo)
 			require.NoError(t, err)
 
-			test.checkFunc(t, p)
+			test.checkInitFunc(t, p)
 
 			require.NoError(t, test.mutator(ctx, p))
 			require.NoError(t, k.writeGlobalPolicy(ctx, "TestDefaultPolicyConfigSet", p))