End an unassigned key with ended and remove no key of the chain (issue 1093)
ADR 0408 section 4 says no key of the chain is removed, a delete marker included. The Backend loses Remove and the Chain RemoveAssignment. An unassignment, and the end of a value only pulled in by another, write the value again with ended set, by compare-and-set; assigning it again writes a fresh standing value over it. A value with ended or a state is no assignment (Assignment.Stands) to the assignment lists, the joins, forgetting a module, seat holdings, the given values, the move and the assignment-changed event.
This commit is contained in:
@@ -35,10 +35,6 @@ 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 (
|
||||
@@ -101,10 +97,10 @@ func (m *Memory) now() time.Time {
|
||||
return time.Now()
|
||||
}
|
||||
|
||||
// last is a key's newest entry, false when it holds none or was removed.
|
||||
// last is a key's newest entry, false when it holds none.
|
||||
func (m *Memory) last(key string) (Entry, bool) {
|
||||
h := m.values[key]
|
||||
if len(h) == 0 || h[len(h)-1].Deleted {
|
||||
if len(h) == 0 {
|
||||
return Entry{}, false
|
||||
}
|
||||
return h[len(h)-1], true
|
||||
@@ -123,26 +119,6 @@ func (m *Memory) Get(_ context.Context, key string) (Entry, bool, error) {
|
||||
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) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
@@ -288,9 +264,7 @@ func (m *Memory) History(_ context.Context, key string) ([]Entry, error) {
|
||||
}
|
||||
out := make([]Entry, 0, len(m.values[key]))
|
||||
for _, e := range m.values[key] {
|
||||
if !e.Deleted {
|
||||
out = append(out, clone(e))
|
||||
}
|
||||
out = append(out, clone(e))
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
+2
-12
@@ -41,7 +41,7 @@ func OnTheBus(ctx context.Context, conn *nats.Conn, holds Holds) (*Chain, error)
|
||||
return New(builds, rollouts, assignments, holds), nil
|
||||
}
|
||||
|
||||
// busBucket is a Backend on a key-value bucket. It calls no delete and no purge: the type has none to call.
|
||||
// busBucket is a Backend on a key-value bucket. It calls no delete and no purge: the type has none to call (ADR 0408 §4).
|
||||
type busBucket struct{ kv jetstream.KeyValue }
|
||||
|
||||
func entryOf(e jetstream.KeyValueEntry) Entry {
|
||||
@@ -76,16 +76,6 @@ 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) {
|
||||
@@ -137,7 +127,7 @@ 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
|
||||
continue // a marker an earlier mirror left; nothing of the chain makes one
|
||||
}
|
||||
out = append(out, entryOf(e))
|
||||
}
|
||||
|
||||
+2
-15
@@ -182,20 +182,8 @@ type Chain struct {
|
||||
Rollouts Bucket[Rollout]
|
||||
Assignments Bucket[Assignment]
|
||||
|
||||
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)
|
||||
rollouts Backend
|
||||
holds Holds
|
||||
}
|
||||
|
||||
// RolloutsOf is the rollouts bucket typed as the controller's plan, which carries more than the chain's Rollout names:
|
||||
@@ -209,7 +197,6 @@ 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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -335,51 +335,57 @@ func TestAnEmptyBucketIsReadAsEmpty(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// **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) {
|
||||
// **An assignment that has ended still stands in the bucket as a value that says so, and nothing in the chain can remove a
|
||||
// key** (ADR 0408 §4, issue 1093): the value with `ended` is no assignment to a reader, and the earlier value stays in
|
||||
// history.
|
||||
func TestAnEndedAssignmentIsNoAssignmentAndStaysInTheBucket(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"})
|
||||
c := InMemory(nil)
|
||||
rev, err := c.Assignments.Create(ctx, "anchor.notes", Assignment{Node: "anchor", Module: "notes", Assigned: at(1), 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)
|
||||
standing, _, _ := c.Assignments.Get(ctx, "anchor.notes")
|
||||
if !standing.Value.Stands() {
|
||||
t.Fatal("a value that has not ended does not stand")
|
||||
}
|
||||
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 {
|
||||
ended := standing.Value
|
||||
ended.Ended = at(2)
|
||||
if _, err := c.Assignments.Update(ctx, "anchor.notes", ended, rev); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, ok, _ := c.Assignments.Get(ctx, "anchor.notes"); ok {
|
||||
t.Fatal("a removed key reads as present")
|
||||
got, found, _ := c.Assignments.Get(ctx, "anchor.notes")
|
||||
if !found || got.Value.Stands() || got.Value.Commit != "c1" || got.Value.Ended.IsZero() {
|
||||
t.Fatalf("the ended value is %+v (found %v)", got.Value, found)
|
||||
}
|
||||
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" {
|
||||
if history, _ := c.Assignments.History(ctx, "anchor.notes"); len(history) != 2 || !history[0].Value.Stands() {
|
||||
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 (Assignment{Node: "a", Module: "m", Assigned: at(1), State: "pending"}).Stands() {
|
||||
t.Fatal("a pending value stands")
|
||||
}
|
||||
if _, ok := any(c.Builds).(interface {
|
||||
Remove(context.Context, string, uint64) error
|
||||
}); ok {
|
||||
t.Fatal("builds can be removed")
|
||||
raw, _ := json.Marshal(ended)
|
||||
if !strings.Contains(string(raw), `"ended":"2026-10-18T02:00:00Z"`) {
|
||||
t.Fatalf("the ended value is %s", raw)
|
||||
}
|
||||
if _, ok := any(c.Rollouts).(interface {
|
||||
// And a value that stands says nothing of either, so a reader of today reads the bytes it did.
|
||||
raw, _ = json.Marshal(Assignment{Node: "a", Module: "m", Assigned: at(1)})
|
||||
if strings.Contains(string(raw), "ended") || strings.Contains(string(raw), "state") {
|
||||
t.Fatalf("a standing value says %s", raw)
|
||||
}
|
||||
// No bucket, and no chain, has a removal.
|
||||
type remover interface {
|
||||
Remove(context.Context, string, uint64) error
|
||||
}); ok {
|
||||
t.Fatal("rollouts can be removed")
|
||||
}
|
||||
for name, x := range map[string]any{"builds": c.Builds, "rollouts": c.Rollouts, "assignments": c.Assignments, "chain": c,
|
||||
"a backend": NewMemory()} {
|
||||
if _, has := x.(remover); has {
|
||||
t.Errorf("%s can remove a key", name)
|
||||
}
|
||||
if _, has := x.(interface {
|
||||
RemoveAssignment(context.Context, string, uint64) error
|
||||
}); has {
|
||||
t.Errorf("%s can remove an assignment", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -186,7 +186,16 @@ type Assignment struct {
|
||||
Commit string `json:"commit,omitempty"`
|
||||
Rollout string `json:"rollout,omitempty"`
|
||||
Since time.Time `json:"since,omitzero"`
|
||||
|
||||
// Ended is when the assignment ended (an unassignment): absent while it stands. **No key of the chain is removed**
|
||||
// (ADR 0408 §4), so an unassigned module keeps its key, saying so; every reader treats a value with Ended, or with a
|
||||
// State, as no assignment. State is, in a later delivery, the pending assignment waiting for its build (issue 1094).
|
||||
Ended time.Time `json:"ended,omitzero"`
|
||||
State string `json:"state,omitempty"`
|
||||
}
|
||||
|
||||
// Stands says the value is an assignment that holds: it has not ended and is not waiting in a state of its own.
|
||||
func (a Assignment) Stands() bool { return a.Ended.IsZero() && a.State == "" }
|
||||
|
||||
// Key is an assignment's key.
|
||||
func (a Assignment) Key() string { return a.Node + "." + a.Module }
|
||||
|
||||
Reference in New Issue
Block a user