Fix stop-loss count and percent check

docs/content/about/release-notes.md

@@ -65,6 +65,7 @@ Default sink HTTP clients now have a 10-second whole-request timeout plus bounde
 * **Behavior change:** The `repl.lag` heartbeat reader now uses `repl-check`; previously it ignored it at every level. With the Blip heartbeat writer and `repl-check` set, an instance that isn't a replica now drops `repl.lag.current`, or reports -1 with `report-not-a-replica=yes`, instead of reporting heartbeat lag.
 * **Behavior change:** Plans that set the `repl.lag` `writer` to `pfs` at one level and `blip` at another now fail to prepare, and the monitor keeps retrying the plan. Previously each level used its own writer. Set the same `writer` at every level, or set it at one level only.
 * Changed `heartbeat.BlipReader.Stop` to wait for the reader to return, which takes up to the heartbeat read timeout (2s) if a heartbeat query is running. Previously the reader could report status after its monitor was stopped and removed, so the removed monitor reappeared in [`GET /status/monitors`]({{< ref "/api/status#get-statusmonitors" >}}).
+* **Behavior change:** Fixed `monitor-loader.stop-loss` to count the net drop in the number of monitors. Previously it compared the number of removed monitors with the number loaded, so a small removal could trigger stop-loss and removing every monitor could not. A percent stop-loss (for example, `50%`) never triggered; it now triggers when the drop is greater than that percent of the monitors loaded before the reload.
 
 ### v2.1.0 (10 Aug 2026)
 

docs/content/config/config-file.md

@@ -178,7 +178,9 @@ The `local` subsection has only two variables:
 |**Valid values**|&bull;&nbsp;"N%" (percentage) where N is an integer between 0 and 100 (exclusive)<br>&bull;&nbsp;"N" where N is an integer greater than 0|
 |**Default value**||
 
-The `stop-loss` variable enables the [stop-lost feature]({{< ref "/monitors/loading#stop-loss" >}}).
+The `stop-loss` variable enables the [stop-loss feature]({{< ref "/monitors/loading#stop-loss" >}}).
+A reload fails if the net drop in the number of monitors is greater than `N` monitors, or greater than `N` percent of the monitors loaded before the reload.
+For example, with 10 monitors loaded and `stop-loss: 30%`, a reload can drop 3 monitors but not 4.
 
 ##### `stop-timeout`
 

docs/content/monitors/loading.md

@@ -54,4 +54,10 @@ Stop-loss prevents reloading from dropping too many MySQL instances due to unrel
 For example, maybe the `LoadMonitors` plugin has a bug that causes it to return zero monitors when there are really 10 monitors.
 Stop-loss would prevent Blip from unloading all monitors, which could have adverse side effects for visibility and alerting.
 
+Stop-loss measures the net drop in the number of monitors: the number loaded before the reload minus the number that would be loaded after it.
+New monitors offset removed monitors, and changed monitors don't count.
+For example, with 10 monitors loaded, a reload that removes 4 monitors and adds 1 drops 3 monitors.
+If the drop is greater than the stop-loss limit, the reload fails and the loaded monitors are not changed.
+A drop equal to the limit is allowed.
+
 Set [`config.monitor-loader.stop-loss`]({{< ref "/config/config-file#stop-loss" >}}) to enable.

monitor/loader.go

@@ -311,20 +311,30 @@ func (ml *Loader) loadAndUnload(ctx context.Context) ([]*stoppingMonitor, error)
 	// Check config.monitor-loader.stop-loss: don't change monitors if there's
 	// a big drop in the number because it might be a false-positive that will
 	// set off alarms when a bunch of metrics fail to report.
-	nBefore := float64(len(ml.repo))
-	nNow := float64(len(diff.removed))
+	//
+	// The loss is the net drop in the number of monitors: nBefore is the number
+	// currently loaded, and nNow is the number that would be loaded after this
+	// change. New monitors offset removed ones; changed monitors are in both
+	// diff.added and diff.changed, so they don't change the number. Stop-loss
+	// is a number or a percent, not both (see blip.StopLoss), and the loss must
+	// be greater than the limit to trigger it.
+	nBefore := len(ml.repo)
+	nNow := nBefore - len(diff.removed) + len(diff.added) - len(diff.changed)
 	if nNow < nBefore {
+		lost := nBefore - nNow
 		var errMsg string
-		if ml.stopLossPercent > 0 {
-			lost := (nBefore - nNow) / nBefore
-			if lost > ml.stopLossPercent {
-				errMsg = fmt.Sprintf("before: %d; now: %d; lost %f%% > limit %f%%", int(nBefore), int(nNow), lost, ml.stopLossPercent)
+		switch {
+		case ml.stopLossNumber > 0:
+			if uint(lost) > ml.stopLossNumber {
+				errMsg = fmt.Sprintf("before: %d; now: %d; lost %d > limit %d", nBefore, nNow, lost, ml.stopLossNumber)
 			}
-		}
-		if ml.stopLossNumber > 0 {
-			lost := uint(nBefore - nNow)
-			if lost > ml.stopLossNumber {
-				errMsg = fmt.Sprintf("before: %d; now: %d; lost %d > limit %d", int(nBefore), int(nNow), lost, ml.stopLossNumber)
+		case ml.stopLossPercent > 0:
+			// stopLossPercent is a whole number (50 for 50%). Compare
+			// lost*100 > limit*before, not lost/before*100 > limit, so a
+			// loss exactly at the limit (3 of 10 for 30%) doesn't round
+			// over it.
+			if float64(lost*100) > ml.stopLossPercent*float64(nBefore) {
+				errMsg = fmt.Sprintf("before: %d; now: %d; lost %.1f%% > limit %.0f%%", nBefore, nNow, float64(lost*100)/float64(nBefore), ml.stopLossPercent)
 			}
 		}
 		if errMsg != "" {

monitor/loader_counters_test.go

@@ -154,10 +154,10 @@ func TestLoaderUnloadedMonitorsHaveNoCounters(t *testing.T) {
 		t.Errorf("counters reported for loaded but not started monitors: %v", got)
 	}
 
-	// Remove 2 of 4 monitors (stop-loss 1) and add a new one: Load makes the
-	// new monitor, then returns ErrStopLoss without adding it.
+	// Remove 3 of 4 monitors and add a new one, a net loss of 2 (stop-loss 1):
+	// Load makes the new monitor, then returns ErrStopLoss without adding it.
 	mux.Lock()
-	configs = []blip.ConfigMonitor{configs[0], configs[1], chronosphereMonitor(newId, "1")}
+	configs = []blip.ConfigMonitor{configs[0], chronosphereMonitor(newId, "1")}
 	mux.Unlock()
 	if err := ml.Load(context.Background()); err != ErrStopLoss {
 		t.Fatalf("Load returned %v, expected ErrStopLoss", err)

monitor/loader_stoploss_test.go

@@ -0,0 +1,161 @@
+// Copyright 2026 Block, Inc.
+
+package monitor_test
+
+import (
+	"context"
+	"fmt"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cashapp/blip/v2"
+	"github.com/cashapp/blip/v2/aws"
+	"github.com/cashapp/blip/v2/dbconn"
+	"github.com/cashapp/blip/v2/event"
+	"github.com/cashapp/blip/v2/monitor"
+	"github.com/cashapp/blip/v2/plan"
+	"github.com/cashapp/blip/v2/test/mock"
+)
+
+func TestLoaderStopLoss(t *testing.T) {
+	// Every case loads 10 monitors, then reloads after removing, adding, and
+	// changing some. Stop-loss is the net drop in the number of monitors:
+	// removed minus added. Changed monitors don't count. The drop must be
+	// greater than the limit to trigger stop-loss.
+	const nBefore = 10
+	tests := []struct {
+		name     string
+		stopLoss string
+		removed  int
+		added    int
+		changed  int
+		stopped  bool   // expect ErrStopLoss
+		msg      string // expected monitors-stoploss event message
+	}{
+		{name: "number below", stopLoss: "3", removed: 2},
+		{name: "number at", stopLoss: "3", removed: 3},
+		{name: "number above", stopLoss: "3", removed: 4, stopped: true, msg: "before: 10; now: 6; lost 4 > limit 3"},
+		{name: "number all", stopLoss: "3", removed: 10, stopped: true, msg: "before: 10; now: 0; lost 10 > limit 3"},
+		{name: "number below with new", stopLoss: "3", removed: 3, added: 1},
+		{name: "number at with new", stopLoss: "3", removed: 4, added: 1},
+		{name: "number above with new", stopLoss: "3", removed: 5, added: 1, stopped: true, msg: "before: 10; now: 6; lost 4 > limit 3"},
+		{name: "number replaced", stopLoss: "3", removed: 5, added: 5},
+		{name: "number changed", stopLoss: "3", changed: 5},
+		{name: "number at with changed", stopLoss: "3", removed: 3, changed: 2},
+		{name: "number above with changed", stopLoss: "3", removed: 4, changed: 2, stopped: true, msg: "before: 10; now: 6; lost 4 > limit 3"},
+		{name: "percent below", stopLoss: "30%", removed: 2},
+		{name: "percent at", stopLoss: "30%", removed: 3},
+		{name: "percent above", stopLoss: "30%", removed: 4, stopped: true, msg: "before: 10; now: 6; lost 40.0% > limit 30%"},
+		{name: "percent all", stopLoss: "30%", removed: 10, stopped: true, msg: "before: 10; now: 0; lost 100.0% > limit 30%"},
+		{name: "percent below with new", stopLoss: "30%", removed: 3, added: 1},
+		{name: "percent at with new", stopLoss: "30%", removed: 4, added: 1},
+		{name: "percent above with new", stopLoss: "30%", removed: 5, added: 1, stopped: true, msg: "before: 10; now: 6; lost 40.0% > limit 30%"},
+		{name: "percent replaced", stopLoss: "30%", removed: 5, added: 5},
+		{name: "percent changed", stopLoss: "30%", changed: 5},
+		{name: "percent at with changed", stopLoss: "30%", removed: 3, changed: 2},
+		{name: "percent above with changed", stopLoss: "30%", removed: 4, changed: 2, stopped: true, msg: "before: 10; now: 6; lost 40.0% > limit 30%"},
+		{name: "disabled", stopLoss: "", removed: 10},
+	}
+	var evmux sync.Mutex
+	var stoplossEvents []event.Event
+	event.Subscribe(mock.EventReceiver{
+		RecvFunc: func(e event.Event) {
+			if e.Event == event.MONITORS_STOPLOSS {
+				evmux.Lock()
+				stoplossEvents = append(stoplossEvents, e)
+				evmux.Unlock()
+			}
+		},
+	})
+	t.Cleanup(event.RemoveSubscribers)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			evmux.Lock()
+			stoplossEvents = nil
+			evmux.Unlock()
+
+			var mux sync.Mutex
+			configs := make([]blip.ConfigMonitor, nBefore)
+			for i := range configs {
+				configs[i] = stopLossMonitor(fmt.Sprintf("stoploss-%d", i))
+			}
+			ml := stopLossLoader(tt.stopLoss, func() []blip.ConfigMonitor {
+				mux.Lock()
+				defer mux.Unlock()
+				return configs
+			})
+			require.NoError(t, ml.Load(context.Background()))
+			require.Equal(t, uint(nBefore), ml.Count())
+			defer func() {
+				for _, mon := range ml.Monitors() {
+					ml.Unload(mon.MonitorId(), true)
+				}
+			}()
+			loaded := map[string]*monitor.Monitor{}
+			for _, mon := range ml.Monitors() {
+				loaded[mon.MonitorId()] = mon
+			}
+
+			// Remove the first monitors, change the next ones, and add new ones.
+			mux.Lock()
+			next := append([]blip.ConfigMonitor{}, configs[tt.removed:]...)
+			for i := 0; i < tt.changed; i++ {
+				next[i].Password = "changed"
+			}
+			for i := 0; i < tt.added; i++ {
+				next = append(next, stopLossMonitor(fmt.Sprintf("stoploss-new-%d", i)))
+			}
+			configs = next
+			mux.Unlock()
+
+			err := ml.Load(context.Background())
+			evmux.Lock()
+			events := stoplossEvents
+			evmux.Unlock()
+			if tt.stopped {
+				assert.ErrorIs(t, err, monitor.ErrStopLoss)
+				// No monitor is removed, replaced, or added.
+				assert.Equal(t, uint(nBefore), ml.Count(), "monitors changed after ErrStopLoss")
+				for id, mon := range loaded {
+					assert.Same(t, mon, ml.Monitor(id), "monitor %s replaced after ErrStopLoss", id)
+				}
+				if assert.Len(t, events, 1) {
+					assert.True(t, events[0].Error)
+					assert.Equal(t, tt.msg, events[0].Message)
+				}
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, uint(nBefore-tt.removed+tt.added), ml.Count())
+				assert.Empty(t, events)
+			}
+		})
+	}
+}
+
+// stopLossLoader returns a Loader with the given stop-loss that loads the
+// monitor configs returned by the configs func. The monitors aren't started.
+func stopLossLoader(stopLoss string, configs func() []blip.ConfigMonitor) *monitor.Loader {
+	cfg := blip.DefaultConfig()
+	cfg.MonitorLoader.StopLoss = stopLoss
+	return monitor.NewLoader(monitor.LoaderArgs{
+		Config:     cfg,
+		Factories:  blip.Factories{DbConn: dbconn.NewConnFactory(nil, nil)},
+		PlanLoader: plan.NewLoader(nil),
+		RDSLoader:  aws.RDSLoader{ClientFactory: mock.RDSClientFactory{}},
+		Plugins: blip.Plugins{
+			LoadMonitors: func(blip.Config) ([]blip.ConfigMonitor, error) { return configs(), nil },
+		},
+	})
+}
+
+func stopLossMonitor(monitorId string) blip.ConfigMonitor {
+	return blip.ConfigMonitor{
+		MonitorId: monitorId,
+		Hostname:  "127.0.0.1:1",
+		Sinks:     map[string]map[string]string{"noop": {}},
+	}
+}