Keep assignments in the assignments bucket and move the rows there at start (issue 1093)

Once chain_moved records the kind, Assign, Unassign and Assigned write and read the keys <node>.<module>, and every join of the store that read assignments is made from the bucket: where a module runs, what is behind, what a machine awaits, a given value due for replacement, taking a module, and whether a module may be forgotten (refused while a standing assignment names it). An unassignment ends the key with a marker and keeps its earlier values, as it removed the row; no ended or pending mark is written yet. The generation rises with each change, and a seat held by an assignment goes with it: the foreign key from seat_holding is dropped by migration 0104 and the controller says both. The provisioner's plan is kept in the values by a pass of the tick that writes that one field by compare-and-set; the assignment-changed event is said from what the bucket holds, whoever wrote it. The start moves the rows with what each machine was last sent, keeping a plan the mirror put in a value.

internal/chain/backend.go

@@ -11,7 +11,9 @@ import (
 // Entry is one key's value at a revision. Caught is set on the entry a watch sends once, after the values the bucket
 // held when it began, and carries no key.
 type Entry struct {
-	Key      string
+	Key string
+	// Deleted marks the end of a key (Remove); it holds no value.
+	Deleted  bool
 	Value    []byte
 	Revision uint64
 	At       time.Time
@@ -33,6 +35,10 @@ type Backend interface {
 	Watch(ctx context.Context) (<-chan Entry, error)
 	// History is a key's earlier values and its current one, oldest first, at most the bucket keeps.
 	History(ctx context.Context, key string) ([]Entry, error)
+	// Remove ends a key at the revision it was read at, with a marker: **its earlier values stay**, read back by History, and
+	// nothing is purged. Used by an unassignment and by nothing else, in the delivery that moves the assignments; the
+	// delivery after it keeps an unassigned key with an end mark and removes this (issue 1096).
+	Remove(ctx context.Context, key string, revision uint64) error
 }
 
 var (
@@ -95,17 +101,46 @@ func (m *Memory) now() time.Time {
 	return time.Now()
 }
 
+// last is a key's newest entry, false when it holds none or was removed.
+func (m *Memory) last(key string) (Entry, bool) {
+	h := m.values[key]
+	if len(h) == 0 || h[len(h)-1].Deleted {
+		return Entry{}, false
+	}
+	return h[len(h)-1], true
+}
+
 func (m *Memory) Get(_ context.Context, key string) (Entry, bool, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	if m.fail != nil {
 		return Entry{}, false, m.fail
 	}
-	h := m.values[key]
-	if len(h) == 0 {
+	e, ok := m.last(key)
+	if !ok {
 		return Entry{}, false, nil
 	}
-	return clone(h[len(h)-1]), true, nil
+	return clone(e), true, nil
+}
+
+func (m *Memory) Remove(_ context.Context, key string, revision uint64) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.fail != nil {
+		return m.fail
+	}
+	e, ok := m.last(key)
+	if !ok || e.Revision != revision {
+		return ErrMoved
+	}
+	m.revision++
+	m.writes++
+	h := append(m.values[key], Entry{Key: key, Deleted: true, Revision: m.revision, At: m.now()})
+	if len(h) > m.keep() {
+		h = h[len(h)-m.keep():]
+	}
+	m.values[key] = h
+	return nil
 }
 
 func (m *Memory) Create(_ context.Context, key string, value []byte) (uint64, error) {
@@ -114,7 +149,7 @@ func (m *Memory) Create(_ context.Context, key string, value []byte) (uint64, er
 	if m.fail != nil {
 		return 0, m.fail
 	}
-	if len(m.values[key]) > 0 {
+	if _, held := m.last(key); held {
 		return 0, ErrExists
 	}
 	return m.write(key, value), nil
@@ -126,8 +161,8 @@ func (m *Memory) Update(_ context.Context, key string, value []byte, revision ui
 	if m.fail != nil {
 		return 0, m.fail
 	}
-	h := m.values[key]
-	if len(h) == 0 || h[len(h)-1].Revision != revision {
+	e, held := m.last(key)
+	if !held || e.Revision != revision {
 		return 0, ErrMoved
 	}
 	return m.write(key, value), nil
@@ -157,7 +192,9 @@ func (m *Memory) Keys(_ context.Context) ([]string, error) {
 	}
 	out := make([]string, 0, len(m.values))
 	for k := range m.values {
-		out = append(out, k)
+		if _, held := m.last(k); held {
+			out = append(out, k)
+		}
 	}
 	sort.Strings(out)
 	return out, nil
@@ -204,8 +241,9 @@ func (m *Memory) Watch(ctx context.Context) (<-chan Entry, error) {
 	}
 	sort.Strings(keys)
 	for _, k := range keys {
-		h := m.values[k]
-		w.q = append(w.q, clone(h[len(h)-1]))
+		if e, held := m.last(k); held {
+			w.q = append(w.q, clone(e))
+		}
 	}
 	w.q = append(w.q, Entry{Caught: true})
 	m.watchers = append(m.watchers, w)
@@ -250,7 +288,9 @@ func (m *Memory) History(_ context.Context, key string) ([]Entry, error) {
 	}
 	out := make([]Entry, 0, len(m.values[key]))
 	for _, e := range m.values[key] {
-		out = append(out, clone(e))
+		if !e.Deleted {
+			out = append(out, clone(e))
+		}
 	}
 	return out, nil
 }

internal/chain/bus.go

@@ -76,6 +76,16 @@ func (b busBucket) Update(ctx context.Context, key string, value []byte, revisio
 	return rev, err
 }
 
+func (b busBucket) Remove(ctx context.Context, key string, revision uint64) error {
+	// A delete marker, never a purge: the key's earlier values stay in the bucket's history.
+	err := b.kv.Delete(ctx, key, jetstream.LastRevision(revision))
+	var apiErr *jetstream.APIError
+	if errors.As(err, &apiErr) && apiErr.ErrorCode == jetstream.JSErrCodeStreamWrongLastSequence {
+		return ErrMoved
+	}
+	return err
+}
+
 func (b busBucket) Keys(ctx context.Context) ([]string, error) {
 	keys, err := b.kv.Keys(ctx)
 	if errors.Is(err, jetstream.ErrNoKeysFound) {
@@ -126,6 +136,9 @@ func (b busBucket) History(ctx context.Context, key string) ([]Entry, error) {
 	}
 	out := make([]Entry, 0, len(all))
 	for _, e := range all {
+		if e.Operation() != jetstream.KeyValuePut {
+			continue
+		}
 		out = append(out, entryOf(e))
 	}
 	return out, nil

internal/chain/chain.go

@@ -182,8 +182,20 @@ type Chain struct {
 	Rollouts    Bucket[Rollout]
 	Assignments Bucket[Assignment]
 
-	rollouts Backend
-	holds    Holds
+	rollouts    Backend
+	assignments Backend
+	holds       Holds
+}
+
+// RemoveAssignment ends an assignment's key with a marker, at the revision it was read at, only by a controller that holds
+// the lease. **The one removal the chain has**, for an unassignment in the delivery that moves the assignments: the
+// key's earlier values stay (history), and nothing is purged. The delivery after it keeps an unassigned key with an end
+// mark in its place and removes this (issue 1096). Builds and rollouts have none.
+func (c *Chain) RemoveAssignment(ctx context.Context, key string, revision uint64) error {
+	if err := c.Assignments.guard(ctx, key, nil); err != nil {
+		return err
+	}
+	return c.assignments.Remove(ctx, key, revision)
 }
 
 // RolloutsOf is the rollouts bucket typed as the controller's plan, which carries more than the chain's Rollout names:
@@ -197,6 +209,7 @@ func New(builds, rollouts, assignments Backend, holds Holds) *Chain {
 		Rollouts:    Bucket[Rollout]{rollouts, holds},
 		Assignments: Bucket[Assignment]{assignments, holds},
 		rollouts:    rollouts,
+		assignments: assignments,
 		holds:       holds,
 	}
 }

internal/chain/chain_test.go

@@ -334,3 +334,52 @@ func TestAnEmptyBucketIsReadAsEmpty(t *testing.T) {
 		t.Fatalf("%v %v", all, err)
 	}
 }
+
+// **The one removal the chain has ends an assignment's key with a marker and keeps its earlier values** (issue 1093,
+// ADR 0408 §5): the key reads as absent, can be made again, builds and rollouts have no removal, and a controller without
+// the lease removes nothing.
+func TestAnAssignmentsKeyEndsWithAMarkerAndKeepsItsHistory(t *testing.T) {
+	ctx := context.Background()
+	var away error
+	c := New(NewMemory(), NewMemory(), NewMemory(), func(context.Context) error { return away })
+	rev, err := c.Assignments.Create(ctx, "anchor.notes", Assignment{Node: "anchor", Module: "notes", Commit: "c1"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	away = errors.New("another controller holds the lease")
+	if err := c.RemoveAssignment(ctx, "anchor.notes", rev); !errors.Is(err, ErrNotHeld) {
+		t.Fatalf("a removal without the lease answered %v", err)
+	}
+	away = nil
+	if err := c.RemoveAssignment(ctx, "anchor.notes", rev+1); !errors.Is(err, ErrMoved) {
+		t.Fatalf("a removal at a revision that moved answered %v", err)
+	}
+	if err := c.RemoveAssignment(ctx, "anchor.notes", rev); err != nil {
+		t.Fatal(err)
+	}
+	if _, ok, _ := c.Assignments.Get(ctx, "anchor.notes"); ok {
+		t.Fatal("a removed key reads as present")
+	}
+	if keys, _ := c.Assignments.Keys(ctx); len(keys) != 0 {
+		t.Fatalf("keys %v", keys)
+	}
+	if all, _ := c.Assignments.All(ctx); len(all) != 0 {
+		t.Fatalf("all %v", all)
+	}
+	if history, _ := c.Assignments.History(ctx, "anchor.notes"); len(history) != 1 || history[0].Value.Commit != "c1" {
+		t.Fatalf("the earlier values are %+v", history)
+	}
+	if _, err := c.Assignments.Create(ctx, "anchor.notes", Assignment{Node: "anchor", Module: "notes"}); err != nil {
+		t.Fatalf("a removed key could not be made again: %v", err)
+	}
+	if _, ok := any(c.Builds).(interface {
+		Remove(context.Context, string, uint64) error
+	}); ok {
+		t.Fatal("builds can be removed")
+	}
+	if _, ok := any(c.Rollouts).(interface {
+		Remove(context.Context, string, uint64) error
+	}); ok {
+		t.Fatal("rollouts can be removed")
+	}
+}