From ee8dd34a220ae893d8ccc927b813c96e141f3f3c Mon Sep 17 00:00:00 2001
From: Andrew Gillis <11790789+gammazero@users.noreply.github.com>
Date: Sat, 12 Oct 2024 08:42:38 -1000
Subject: [PATCH] Unfreeze even if freezing is disabled. (#2671)

Freezing may be disabled after an indexer becomes frozen to prevent the indexer from freezing again. Unfreezing the indexer should still work even if freezing is disabled. This PR makes this possible, whereass previously an indexer could not be unfrozen if freezing was disabled.

Closes #2670

This PR also allows disk usage to be queried even when freezing is disabled.
---
 config/indexer.go               | 10 +++---
 internal/freeze/freezer.go      | 60 ++++++++++++++++++++++++++-------
 internal/freeze/freezer_test.go | 22 ++++++++++--
 internal/registry/errors.go     |  1 -
 internal/registry/registry.go   | 21 +++---------
 server/admin/handler.go         |  3 +-
 6 files changed, 79 insertions(+), 38 deletions(-)

diff --git a/config/indexer.go b/config/indexer.go
index b03a4fc13..8e3fe70ac 100644
--- a/config/indexer.go
+++ b/config/indexer.go
@@ -27,9 +27,11 @@ type Indexer struct {
 	DHStoreClusterURLs []string
 	// DHStoreHttpClientTimeout is a timeout for the DHStore http client
 	DHStoreHttpClientTimeout Duration
-	// FreezeAtPercent is the percent used, of the file system that
-	// ValueStoreDir is on, at which to trigger the indexer to enter frozen
-	// mode. A zero value uses the default. A negative value disables freezing.
+	// FreezeAtPercent is the filesystem percent used at which to trigger the
+	// indexer to enter frozen mode. The indexer checks the file system of
+	// ValueStoreDir, Datastore.Dir, and Datastore.TmpDir and enters frozen
+	// mode if any one's usage is at or above FreezeAtPercent. A zero value
+	// uses the default. A negative value disables freezing.
 	FreezeAtPercent float64
 	// ShutdownTimeout is the duration that a graceful shutdown has to complete
 	// before the daemon process is terminated. If unset or zero, configures no
@@ -62,7 +64,7 @@ func NewIndexer() Indexer {
 		CacheSize:            300000,
 		PebbleBlockCacheSize: 1 << 30, // 1 Gi
 		ConfigCheckInterval:  Duration(30 * time.Second),
-		FreezeAtPercent:      90.0,
+		FreezeAtPercent:      95.0,
 		ShutdownTimeout:      0,
 		ValueStoreDir:        "valuestore",
 		ValueStoreType:       "pebble",
diff --git a/internal/freeze/freezer.go b/internal/freeze/freezer.go
index 5b4e60a56..43e2243b2 100644
--- a/internal/freeze/freezer.go
+++ b/internal/freeze/freezer.go
@@ -2,6 +2,7 @@ package freeze
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strconv"
 	"time"
@@ -14,6 +15,8 @@ import (
 var log = logging.Logger("indexer/freezer")
 
 const (
+	defaultFreezeAtPercent = 99.0
+
 	frozenKey = "/freeze/frozen"
 
 	maxCheckInterval = time.Hour
@@ -27,6 +30,8 @@ const (
 	logCriticalRemaining = 2.0
 )
 
+var ErrNoFreeze = errors.New("freezing disabled")
+
 // Freezer monitors disk usage and triggers a freeze if the usage reaches a
 // specified threshold.
 type Freezer struct {
@@ -37,6 +42,7 @@ type Freezer struct {
 	freezeAtStr string
 	freezeFunc  func() error
 	frozen      chan struct{}
+	noFreeze    bool
 	trigger     chan struct{}
 	triggerErr  chan error
 	paths       []string
@@ -48,14 +54,27 @@ func New(dirPaths []string, freezeAtPercent float64, dstore datastore.Datastore,
 	if err != nil {
 		return nil, err
 	}
+
 	f := &Freezer{
-		dstore:      dstore,
-		freezeAt:    freezeAtPercent,
-		freezeAtStr: fmt.Sprintf("%s%%", strconv.FormatFloat(freezeAtPercent, 'f', -1, 64)),
-		freezeFunc:  freezeFunc,
-		frozen:      make(chan struct{}),
-		paths:       dirPaths,
+		dstore: dstore,
+		frozen: make(chan struct{}),
+		paths:  dirPaths,
 	}
+
+	if freezeAtPercent < 0 {
+		f.noFreeze = true
+	} else {
+		if freezeAtPercent == 0 {
+			freezeAtPercent = defaultFreezeAtPercent
+		}
+
+		f.freezeAt = freezeAtPercent
+		f.freezeFunc = freezeFunc
+		f.freezeAtStr = fmt.Sprintf("%s%%", strconv.FormatFloat(freezeAtPercent, 'f', -1, 64))
+
+		log.Infow("freezing enabled", "freezeAt", f.freezeAtStr)
+	}
+
 	frozen, err := f.loadFrozenState()
 	if err != nil {
 		return nil, err
@@ -66,6 +85,10 @@ func New(dirPaths []string, freezeAtPercent float64, dstore datastore.Datastore,
 		return f, nil
 	}
 
+	if f.noFreeze {
+		return f, nil
+	}
+
 	// If not frozen, check disk usage and start monitor.
 	nextCheck, frozen, err := f.check()
 	if err != nil {
@@ -85,6 +108,9 @@ func New(dirPaths []string, freezeAtPercent float64, dstore datastore.Datastore,
 
 // Freeze manually triggers the indexer to enter frozen mode.
 func (f *Freezer) Freeze() error {
+	if f.noFreeze {
+		return ErrNoFreeze
+	}
 	select {
 	case f.trigger <- struct{}{}:
 		return <-f.triggerErr
@@ -108,6 +134,9 @@ func (f *Freezer) CheckNow() bool {
 	if f.Frozen() {
 		return true
 	}
+	if f.noFreeze {
+		return false
+	}
 	checkDone := make(chan struct{})
 	select {
 	case f.checkNow <- checkDone:
@@ -165,13 +194,20 @@ func Unfreeze(ctx context.Context, dirPaths []string, freezeAtPercent float64, d
 		return nil
 	}
 
-	for _, dirPath := range dirPaths {
-		du, err := disk.Usage(dirPath)
-		if err != nil {
-			return fmt.Errorf("cannot get disk usage for freeze check at path %q: %w", dirPath, err)
+	// If freezing enabled, then check for sufficient space to unfreeze.
+	if freezeAtPercent >= 0 {
+		if freezeAtPercent == 0 {
+			freezeAtPercent = defaultFreezeAtPercent
 		}
-		if du.Percent >= freezeAtPercent {
-			return fmt.Errorf("cannot unfreeze: disk usage above %f", freezeAtPercent)
+
+		for _, dirPath := range dirPaths {
+			du, err := disk.Usage(dirPath)
+			if err != nil {
+				return fmt.Errorf("cannot get disk usage for freeze check at path %q: %w", dirPath, err)
+			}
+			if du.Percent >= freezeAtPercent {
+				return fmt.Errorf("cannot unfreeze: disk usage above %f", freezeAtPercent)
+			}
 		}
 	}
 
diff --git a/internal/freeze/freezer_test.go b/internal/freeze/freezer_test.go
index 1e5edfb35..6dc6b0049 100644
--- a/internal/freeze/freezer_test.go
+++ b/internal/freeze/freezer_test.go
@@ -48,26 +48,26 @@ func TestCheckFreeze(t *testing.T) {
 	}
 	f.Close()
 
+	// Check that freeze does not happen when enough storage.
 	f, err = freeze.New(dirs, du.Percent*2.0, dstore, freezeFunc)
 	require.NoError(t, err)
 	require.False(t, f.Frozen())
 	f.Close()
-
 	require.Zero(t, freezeCount)
 
+	// Check that freeze happens when insufficient storage.
 	f, err = freeze.New(dirs, du.Percent/2.0, dstore, freezeFunc)
 	require.NoError(t, err)
 	require.True(t, f.Frozen())
 	require.True(t, f.CheckNow())
 	f.Close()
-
 	require.Equal(t, 1, freezeCount)
 
+	// Check that freeze does not happen again.
 	f, err = freeze.New(dirs, du.Percent/2.0, dstore, freezeFunc)
 	require.NoError(t, err)
 	require.True(t, f.Frozen())
 	f.Close()
-
 	require.Equal(t, 1, freezeCount)
 
 	// Unfreeze no directories.
@@ -90,6 +90,16 @@ func TestCheckFreeze(t *testing.T) {
 	// Unfreeze already unfrozen.
 	err = freeze.Unfreeze(context.Background(), dirs, du.Percent*2.0, dstore)
 	require.NoError(t, err)
+
+	// Freeze and check that unfreeze works when freeze is disabled.
+	f, err = freeze.New(dirs, du.Percent/2.0, dstore, freezeFunc)
+	require.NoError(t, err)
+	require.True(t, f.Frozen())
+	f.Close()
+	require.Equal(t, 2, freezeCount)
+
+	err = freeze.Unfreeze(context.Background(), dirs, -1, dstore)
+	require.NoError(t, err)
 }
 
 func TestManualFreeze(t *testing.T) {
@@ -121,5 +131,11 @@ func TestManualFreeze(t *testing.T) {
 	require.NoError(t, f.Freeze())
 	f.Close()
 
+	f, err = freeze.New(dirs, -1, dstore, freezeFunc)
+	require.NoError(t, err)
+	require.ErrorIs(t, f.Freeze(), freeze.ErrNoFreeze)
+	require.True(t, f.Frozen())
+	f.Close()
+
 	require.Equal(t, 1, freezeCount)
 }
diff --git a/internal/registry/errors.go b/internal/registry/errors.go
index 36a068282..901b5ddec 100644
--- a/internal/registry/errors.go
+++ b/internal/registry/errors.go
@@ -10,7 +10,6 @@ var (
 	ErrMissingProviderAddr = errors.New("advertisement missing provider address")
 	ErrNotAllowed          = errors.New("peer not allowed by policy")
 	ErrNoDiscovery         = errors.New("discovery not available")
-	ErrNoFreeze            = errors.New("freeze not configured")
 	ErrNotVerified         = errors.New("provider cannot be verified")
 	ErrPublisherNotAllowed = errors.New("publisher not allowed by policy")
 	ErrTooSoon             = errors.New("not enough time since previous discovery")
diff --git a/internal/registry/registry.go b/internal/registry/registry.go
index 72133da97..727ee4fec 100644
--- a/internal/registry/registry.go
+++ b/internal/registry/registry.go
@@ -324,11 +324,9 @@ func New(ctx context.Context, cfg config.Discovery, dstore datastore.Datastore,
 		}
 	}
 
-	if opts.freezeAtPercent >= 0 {
-		r.freezer, err = freeze.New(opts.freezeDirs, opts.freezeAtPercent, dstore, r.freeze)
-		if err != nil {
-			return nil, fmt.Errorf("cannot create freezer: %s", err)
-		}
+	r.freezer, err = freeze.New(opts.freezeDirs, opts.freezeAtPercent, dstore, r.freeze)
+	if err != nil {
+		return nil, fmt.Errorf("cannot create freezer: %s", err)
 	}
 
 	if cfg.PollInterval != 0 {
@@ -411,9 +409,7 @@ func makePollOverrideMap(poll polling, cfgPollOverrides []config.Polling) (map[p
 // Close stops the registry and waits for polling to finish.
 func (r *Registry) Close() {
 	r.closeOnce.Do(func() {
-		if r.freezer != nil {
-			r.freezer.Close()
-		}
+		r.freezer.Close()
 		close(r.closing)
 		<-r.tmpBlockCheckDone
 		if r.pollDone != nil {
@@ -1114,9 +1110,6 @@ func (r *Registry) CheckSequence(peerID peer.ID, seq uint64) error {
 //
 // The registry in not frozen directly, but the Freezer is triggered instead.
 func (r *Registry) Freeze() error {
-	if r.freezer == nil {
-		return ErrNoFreeze
-	}
 	return r.freezer.Freeze()
 }
 
@@ -1168,16 +1161,10 @@ func (r *Registry) freezeProviders() error {
 
 // Frozen returns true if indexer is frozen.
 func (r *Registry) Frozen() bool {
-	if r.freezer == nil {
-		return false
-	}
 	return r.freezer.Frozen()
 }
 
 func (r *Registry) ValueStoreUsage() (*disk.UsageStats, error) {
-	if r.freezer == nil {
-		return nil, ErrNoFreeze
-	}
 	return r.freezer.Usage()
 }
 
diff --git a/server/admin/handler.go b/server/admin/handler.go
index 0a261c562..a664bb69b 100644
--- a/server/admin/handler.go
+++ b/server/admin/handler.go
@@ -16,6 +16,7 @@ import (
 	"github.com/ipfs/go-cid"
 	"github.com/ipni/go-indexer-core"
 	"github.com/ipni/storetheindex/admin/model"
+	"github.com/ipni/storetheindex/internal/freeze"
 	"github.com/ipni/storetheindex/internal/httpserver"
 	"github.com/ipni/storetheindex/internal/ingest"
 	"github.com/ipni/storetheindex/internal/registry"
@@ -468,7 +469,7 @@ func (h *adminHandler) freeze(w http.ResponseWriter, r *http.Request) {
 
 	err := h.reg.Freeze()
 	if err != nil {
-		if errors.Is(err, registry.ErrNoFreeze) {
+		if errors.Is(err, freeze.ErrNoFreeze) {
 			log.Infow("Cannot freeze indexer", "reason", err)
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return