Repository: kopia/kopia
Pull request #3646 (2024-02-13 00:58:51)

fix(general): disable writes on index loads
Change default `allowWritesOnIndexLoad` to false when the corresponding environment variable is unset.

Add unit test to ensure epoch is not advanced on index loads by default

Followup to #3645

Ref:
- #3638
- #3639


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/kopia/kopia/pull/3646?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`1eabe17`)](https://app.codecov.io/gh/kopia/kopia/commit/1eabe173b5f4cd083287c0eb208bb696437368df?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) 77.17% compared to head [(`8bafe12`)](https://app.codecov.io/gh/kopia/kopia/pull/3646?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) 76.74%.


<details><summary>Additional details and impacted files</summary>


```diff
@@                           Coverage Diff                            @@
##           fix/no-index-cleanup-on-load-env-var    #3646      +/-   ##
========================================================================
- Coverage                                 77.17%   76.74%   -0.44%     
========================================================================
  Files                                       470      470              
  Lines                                     28446    28444       -2     
========================================================================
- Hits                                      21954    21830     -124     
- Misses                                     4568     4681     +113     
- Partials                                   1924     1933       +9     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/kopia/kopia/pull/3646?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).


Patch:
diff --git a/internal/epoch/epoch_manager.go b/internal/epoch/epoch_manager.go
index ddd4bf8d..b22b8bc7 100644
--- a/internal/epoch/epoch_manager.go
+++ b/internal/epoch/epoch_manager.go
@@ -1133,11 +1133,6 @@ func rangeCheckpointBlobPrefix(epoch1, epoch2 int) blob.ID {
 func allowWritesOnIndexLoad() bool {
 	v := strings.ToLower(os.Getenv("KOPIA_ALLOW_WRITE_ON_INDEX_LOAD"))
 
-	if v == "" {
-		// temporary default to be changed once index cleanup is performed on maintenance
-		return true
-	}
-
 	return v == "true" || v == "1"
 }
 


Test patch:
diff --git a/internal/epoch/epoch_manager_test.go b/internal/epoch/epoch_manager_test.go
index 732ebc61..3926da97 100644
--- a/internal/epoch/epoch_manager_test.go
+++ b/internal/epoch/epoch_manager_test.go
@@ -408,6 +408,89 @@ func TestIndexEpochManager_NoCompactionInReadOnly(t *testing.T) {
 	assert.Nil(t, loadedErr.Load(), "refreshing read-only index")
 }
 
+func TestNoEpochAdvanceOnIndexRead(t *testing.T) {
+	const epochs = 3
+
+	t.Parallel()
+
+	ctx := testlogging.Context(t)
+	te := newTestEnv(t)
+
+	p, err := te.mgr.getParameters(ctx)
+	require.NoError(t, err)
+
+	count := p.GetEpochAdvanceOnCountThreshold()
+	minDuration := p.MinEpochDuration
+
+	cs, err := te.mgr.Current(ctx)
+	require.NoError(t, err)
+	require.Equal(t, 0, cs.WriteEpoch, "write epoch mismatch")
+
+	// Write enough index blobs such that the next time the manager loads
+	// indexes it should attempt to advance the epoch.
+	// Write exactly the number of index blobs that will cause it to advance so
+	// we can keep track of which one is the current epoch.
+	for j := 0; j < epochs; j++ {
+		for i := 0; i < count-1; i++ {
+			te.mustWriteIndexFiles(ctx, t, newFakeIndexWithEntries(i))
+		}
+
+		te.ft.Advance(3*minDuration + time.Second)
+		te.mustWriteIndexFiles(ctx, t, newFakeIndexWithEntries(count-1))
+		// this could advance the epoch on write
+		te.mustWriteIndexFiles(ctx, t, newFakeIndexWithEntries(count-1))
+	}
+
+	te.mgr.Invalidate()
+	cs, err = te.mgr.Current(ctx)
+	require.NoError(t, err)
+
+	te.mgr.Flush() // wait for background work
+
+	// get written lastWriteEpoch markers if any
+	var (
+		lastWriteEpoch int
+		epochMarkers   []blob.ID
+		deletedMarker  blob.ID
+	)
+
+	te.st.ListBlobs(ctx, EpochMarkerIndexBlobPrefix, func(bm blob.Metadata) error {
+		epochMarkers = append(epochMarkers, bm.BlobID)
+
+		return nil
+	})
+
+	t.Log("epoch marker blobs:", epochMarkers)
+
+	if emLen := len(epochMarkers); emLen > 0 {
+		var ok bool // to prevent shadowing 'lastWriteEpoch' below
+
+		deletedMarker = epochMarkers[emLen-1]
+		lastWriteEpoch, ok = epochNumberFromBlobID(deletedMarker)
+
+		require.True(t, ok, "could not parse epoch from marker blob")
+	}
+
+	require.Equal(t, 0, lastWriteEpoch, "epoch should NOT have advanced")
+
+	// reload indexes
+	te.mgr.Invalidate()
+
+	cs, err = te.mgr.Current(ctx)
+	require.NoError(t, err)
+
+	// wait for any background work, there shouldn't be any
+	te.mgr.backgroundWork.Wait()
+
+	require.Equal(t, 0, cs.WriteEpoch, "epoch should NOT have advanced")
+
+	te.st.ListBlobs(ctx, EpochMarkerIndexBlobPrefix, func(bm blob.Metadata) error {
+		t.Fatal("deleted epoch marker should NOT be found in the store:", deletedMarker)
+
+		return nil
+	})
+}
+
 func TestRefreshRetriesIfTakingTooLong(t *testing.T) {
 	te := newTestEnv(t)