Put deleted_at in the event-tier indexes so SQLite uses them

GORM adds deleted_at IS NULL to these queries, and SQLite, which has no
table statistics here, preferred the deleted_at index to the new
single-column ones wherever a column is matched against several values
or compared with <. The event log's attempt load, retention's lookups
of expired events and their deliveries, and the queue-depth sampler
still read every live row. Each index now also covers deleted_at,
first in the events index because created_at is compared with <;
events keeps its created_at index for retention's final delete. A test
checks SQLite's plan for each statement as GORM builds it.

Model: opus-5-5

README.md

@@ -1761,19 +1761,25 @@ retries) is individually logged for full observability.
 
 #### Event-tier indexes
 
-Beyond the primary keys, the per-webhook event databases carry secondary
+These indexes on the per-webhook event databases are declared in the model
-indexes on the columns the background work reads by, each created by
+tags, so `AutoMigrate` creates them on a fresh and on an existing database:
-`AutoMigrate` on a fresh and on an existing database:
 
-| Column                         | Serves |
+| Table              | Columns                     | Serves |
-| ------------------------------ | ------ |
+| ------------------ | --------------------------- | ------ |
-| `deliveries.status`            | The recovery and sweep queries that select deliveries by status once a minute |
+| `deliveries`       | `status`, `deleted_at`      | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status |
-| `deliveries.event_id`          | Loading a page of the event log, which reads deliveries by event |
+| `deliveries`       | `event_id`, `deleted_at`    | The event log, which loads each event's deliveries, and retention, which selects and deletes the deliveries of expired events |
-| `delivery_results.delivery_id` | Loading a page of the event log, which reads results by delivery |
+| `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events |
-| `events.created_at`            | Retention, which deletes events by age |
+| `events`           | `deleted_at`, `created_at`  | Retention, which selects expired events by age |
+| `events`           | `created_at`                | Retention's delete of the expired events themselves |
 
-The `events.resubmitted_from_id` column is also indexed, to resolve the
+GORM's soft delete adds `deleted_at IS NULL` to these queries; retention's
-resubmit relationship both ways in the event log.
+deletes leave it out, but their lookups of expired rows keep it. SQLite keeps
+no statistics on these tables, and without them it rates the `deleted_at`
+index, which every live row matches, above an index on a column matched
+against several values or compared with `<`. So every index but the last also
+covers `deleted_at`. It comes second, so that retention's deletes can use the
+index without it, except in `events`, where `created_at` is compared with `<`
+and SQLite narrows by a `<` only on the last column it uses.
 
 #### Common Fields
 

internal/database/event_tier_indexes_test.go

@@ -2,37 +2,33 @@ package database_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
+	"time"
 
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
 )
 
-// indexedColumn names a secondary index by the model and struct field
-// GORM derives the index name from.
-type indexedColumn struct {
-	model any
-	field string
-}
-
 // TestWebhookDBManager_OpenAddsEventTierIndexes verifies that opening a
-// per-webhook database that predates these indexes creates them, so the
+// per-webhook database that predates these indexes creates them. It
-// queries that read by those columns stop scanning whole tables. It
 // stands in for an older database file by dropping the indexes
 // AutoMigrate just created, then reopening the same file.
 func TestWebhookDBManager_OpenAddsEventTierIndexes(t *testing.T) {
 	t.Parallel()
 
-	// The columns the background work reads by: the recovery and sweep
+	indexes := []struct {
-	// queries (status), the event log (event_id and delivery_id) and
+		model any
-	// retention (created_at).
+		name  string
-	eventTierIndexes := []indexedColumn{
+	}{
-		{&database.Delivery{}, "Status"},
+		{&database.Delivery{}, "idx_deliveries_status"},
-		{&database.Delivery{}, "EventID"},
+		{&database.Delivery{}, "idx_deliveries_event_id"},
-		{&database.DeliveryResult{}, "DeliveryID"},
+		{&database.DeliveryResult{}, "idx_delivery_results_delivery_id"},
-		{&database.Event{}, "CreatedAt"},
+		{&database.Event{}, "idx_events_deleted_at_created_at"},
+		{&database.Event{}, "idx_events_created_at"},
 	}
 
 	mgr, lc := setupTestWebhookDBManager(t)
@@ -47,14 +43,14 @@ func TestWebhookDBManager_OpenAddsEventTierIndexes(t *testing.T) {
 	require.NoError(t, err)
 
 	// A fresh database has them.
-	for _, ix := range eventTierIndexes {
+	for _, ix := range indexes {
-		require.True(t, db.Migrator().HasIndex(ix.model, ix.field))
+		require.True(t, db.Migrator().HasIndex(ix.model, ix.name))
 	}
 
 	// Stand in for a database file created before the indexes existed.
-	for _, ix := range eventTierIndexes {
+	for _, ix := range indexes {
-		require.NoError(t, db.Migrator().DropIndex(ix.model, ix.field))
+		require.NoError(t, db.Migrator().DropIndex(ix.model, ix.name))
-		require.False(t, db.Migrator().HasIndex(ix.model, ix.field))
+		require.False(t, db.Migrator().HasIndex(ix.model, ix.name))
 	}
 
 	// Drop the cached connection so the next open reopens the file and
@@ -64,9 +60,107 @@ func TestWebhookDBManager_OpenAddsEventTierIndexes(t *testing.T) {
 	db, err = mgr.GetDB(webhookID)
 	require.NoError(t, err)
 
-	for _, ix := range eventTierIndexes {
+	for _, ix := range indexes {
-		assert.True(t, db.Migrator().HasIndex(ix.model, ix.field),
+		assert.True(t, db.Migrator().HasIndex(ix.model, ix.name),
-			"opening the existing database should create the index on %s",
+			"opening the existing database should create %s", ix.name)
-			ix.field)
+	}
+}
+
+// TestEventTierQueriesUseTheirIndexes verifies that the statements the
+// indexes are for use them. GORM builds each statement in a dry run as
+// the code named above it does, soft-delete condition included, and
+// SQLite, which keeps no statistics on these tables, must plan to seek
+// on each index listed by the columns in parentheses.
+func TestEventTierQueriesUseTheirIndexes(t *testing.T) {
+	t.Parallel()
+
+	mgr, lc := setupTestWebhookDBManager(t)
+	ctx := context.Background()
+	require.NoError(t, lc.Start(ctx))
+
+	defer func() { require.NoError(t, lc.Stop(ctx)) }()
+
+	db, err := mgr.GetDB(uuid.New().String())
+	require.NoError(t, err)
+
+	dry := db.Session(&gorm.Session{DryRun: true})
+	ids := []string{
+		uuid.New().String(), uuid.New().String(), uuid.New().String(),
+	}
+	cutoff := time.Now()
+
+	var (
+		deliveries []database.Delivery
+		results    []database.DeliveryResult
+		depths     []struct{ Depth int }
+	)
+
+	byStatus := "idx_deliveries_status (status=? AND deleted_at=?)"
+	byEvent := "idx_deliveries_event_id (event_id=? AND deleted_at=?)"
+	byAge := "idx_events_deleted_at_created_at (deleted_at=? AND created_at<?)"
+
+	// The delivery engine: recovery and the retry sweep, the sweep for
+	// stranded pending deliveries, and the queue depth count.
+	assertPlanUses(t, db, dry.Where(
+		"status = ?", database.DeliveryStatusRetrying,
+	).Find(&deliveries), byStatus)
+	assertPlanUses(t, db, dry.Where(
+		"status = ? AND updated_at < ?",
+		database.DeliveryStatusPending, cutoff,
+	).Limit(500).Find(&deliveries), byStatus)
+	assertPlanUses(t, db, dry.Model(&database.Delivery{}).
+		Select("target_id", "status", "count(*) as depth").
+		Where("status IN ?", []database.DeliveryStatus{
+			database.DeliveryStatusPending,
+			database.DeliveryStatusRetrying,
+		}).Group("target_id, status").Find(&depths), byStatus)
+
+	// The event log: each event's deliveries, then their attempts
+	// (loadEventsWithDeliveries, loadDeliveryResults).
+	assertPlanUses(t, db, dry.Where("event_id = ?", ids[0]).
+		Find(&deliveries), byEvent)
+	assertPlanUses(t, db, dry.Where("delivery_id IN ?", ids).
+		Order("attempt_num ASC").Find(&results),
+		"idx_delivery_results_delivery_id (delivery_id=? AND deleted_at=?)")
+
+	// Retention's three deletes (reapExpired), whose subqueries are built
+	// afresh for each statement as it builds them.
+	expiredEventIDs := func() *gorm.DB {
+		return dry.Model(&database.Event{}).Select("id").
+			Where("created_at < ?", cutoff)
+	}
+
+	assertPlanUses(t, db, dry.Unscoped().Where(
+		"delivery_id IN (?)", dry.Model(&database.Delivery{}).
+			Select("id").Where("event_id IN (?)", expiredEventIDs()),
+	).Delete(&database.DeliveryResult{}),
+		"idx_delivery_results_delivery_id (delivery_id=?)", byEvent, byAge)
+	assertPlanUses(t, db, dry.Unscoped().Where(
+		"event_id IN (?)", expiredEventIDs(),
+	).Delete(&database.Delivery{}),
+		"idx_deliveries_event_id (event_id=?)", byAge)
+	assertPlanUses(t, db, dry.Unscoped().Where(
+		"created_at < ?", cutoff,
+	).Delete(&database.Event{}), "idx_events_created_at (created_at<?)")
+}
+
+// assertPlanUses asserts that SQLite's plan for a statement GORM built
+// in a dry run, run with the same SQL and arguments GORM would send,
+// names each of the given indexes.
+func assertPlanUses(
+	t *testing.T, db, built *gorm.DB, indexes ...string,
+) {
+	t.Helper()
+
+	var plan []struct{ Detail string }
+
+	require.NoError(t, db.Raw(
+		"EXPLAIN QUERY PLAN "+built.Statement.SQL.String(),
+		built.Statement.Vars...,
+	).Scan(&plan).Error)
+
+	for _, index := range indexes {
+		assert.Contains(t, fmt.Sprint(plan), index,
+			built.Statement.SQL.String())
 	}
 }

internal/database/model_delivery.go

@@ -1,5 +1,7 @@
 package database
 
+import "gorm.io/gorm"
+
 // DeliveryStatus represents the status of a delivery
 type DeliveryStatus string
 
@@ -29,12 +31,19 @@ func (s DeliveryStatus) Terminal() bool {
 }
 
 // Delivery represents a delivery attempt for an event to a target
+//
+//nolint:lll // a struct tag cannot wrap
 type Delivery struct {
 	BaseModel
 
-	EventID  string         `gorm:"type:uuid;not null;index"         json:"eventId"`
+	EventID  string         `gorm:"type:uuid;not null;index:idx_deliveries_event_id,priority:1"       json:"eventId"`
-	TargetID string         `gorm:"type:uuid;not null"               json:"targetId"`
+	TargetID string         `gorm:"type:uuid;not null"                                                json:"targetId"`
-	Status   DeliveryStatus `gorm:"not null;default:'pending';index" json:"status"`
+	Status   DeliveryStatus `gorm:"not null;default:'pending';index:idx_deliveries_status,priority:1" json:"status"`
+
+	// DeletedAt repeats the BaseModel field only to be the second column
+	// of the event_id and status indexes, for the reason DeliveryResult
+	// gives.
+	DeletedAt gorm.DeletedAt `gorm:"index:idx_deliveries_event_id,priority:2;index:idx_deliveries_status,priority:2" json:"deletedAt,omitzero"`
 
 	// Relations
 	Event           Event            `json:"event,omitzero"`

internal/database/model_delivery_result.go

@@ -1,14 +1,25 @@
 package database
 
+import "gorm.io/gorm"
+
 // DeliveryResult represents the result of a delivery attempt
+//
+//nolint:lll // a struct tag cannot wrap
 type DeliveryResult struct {
 	BaseModel
 
-	DeliveryID   string `gorm:"type:uuid;not null;index" json:"deliveryId"`
+	// DeliveryID and DeletedAt make up one index, in that order.
-	AttemptNum   int    `gorm:"not null"                 json:"attemptNum"`
+	// DeletedAt repeats the BaseModel field only to join it: GORM adds
+	// "deleted_at IS NULL" to almost every query, and where a column is
+	// matched against several values SQLite otherwise reads through the
+	// deleted_at index, which every live row matches.
+	DeliveryID string         `gorm:"type:uuid;not null;index:idx_delivery_results_delivery_id,priority:1" json:"deliveryId"`
+	DeletedAt  gorm.DeletedAt `gorm:"index:idx_delivery_results_delivery_id,priority:2"                    json:"deletedAt,omitzero"`
+
+	AttemptNum   int    `gorm:"not null"             json:"attemptNum"`
 	Success      bool   `json:"success"`
 	StatusCode   int    `json:"statusCode,omitempty"`
-	ResponseBody string `gorm:"type:text"                json:"responseBody,omitempty"`
+	ResponseBody string `gorm:"type:text"            json:"responseBody,omitempty"`
 	Error        string `json:"error,omitempty"`
 	Duration     int64  `json:"durationMs"` // Duration in milliseconds
 

internal/database/model_event.go

@@ -1,16 +1,26 @@
 package database
 
-import "time"
+import (
+	"time"
+
+	"gorm.io/gorm"
+)
 
 // Event represents a captured webhook event
+//
+//nolint:lll // a struct tag cannot wrap
 type Event struct {
 	BaseModel
 
-	// CreatedAt overrides BaseModel.CreatedAt only to add an index:
+	// CreatedAt and DeletedAt repeat the BaseModel fields only to index
-	// retention deletes events by age, so events.created_at is queried
+	// them for retention, which finds events by age. Its lookups carry
-	// on every sweep. The other tables keep the unindexed BaseModel
+	// GORM's "deleted_at IS NULL" (see DeliveryResult) and compare
-	// field.
+	// created_at with <, so their index has deleted_at first: SQLite
-	CreatedAt time.Time `gorm:"index" json:"createdAt"`
+	// narrows by a < only on the last column it uses. Its final delete
+	// has no deleted_at condition and uses the index on created_at
+	// alone. The other tables keep the unindexed BaseModel created_at.
+	CreatedAt time.Time      `gorm:"index;index:idx_events_deleted_at_created_at,priority:2" json:"createdAt"`
+	DeletedAt gorm.DeletedAt `gorm:"index:idx_events_deleted_at_created_at,priority:1"       json:"deletedAt,omitzero"`
 
 	WebhookID    string `gorm:"type:uuid;not null" json:"webhookId"`
 	EntrypointID string `gorm:"type:uuid;not null" json:"entrypointId"`