From 9c548753b800dd33bc3b2e096ed098c2c138ec70 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 12:33:41 +0200 Subject: [PATCH] go: say an Increment error other than a lost write may have spent its number, refuse null, and prove the writers collided The review of 2026-10-10: a timeout can come after the write landed, so a caller tolerates gaps; a null value is no counter; and a contention test where no write was lost would prove nothing. --- go/state.go | 10 ++++++++-- go/state_counter_test.go | 22 +++++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/go/state.go b/go/state.go index 6d7abdb..7197cab 100644 --- a/go/state.go +++ b/go/state.go @@ -80,8 +80,11 @@ func (s KeptState) Delete(key string) error { // value. Read, then written only if the key is still at the revision read (Create when it had none, Update // when it had one), so of any number of writers each number is handed out once and no increment is lost. // A write another writer won is read again and tried again, at most attempts times in all; past that the -// error is ErrChanged and nothing was counted. Any other refusal is returned at once, as is a value that is -// not a whole number: nothing is ever reported counted that was not written. +// error is ErrChanged and nothing was counted. Any other error is returned at once, as is a value that is +// not a whole number (null included): nothing is ever reported counted that was not written. The reverse +// does not hold: an error that is not ErrChanged — a timeout above all — may come after the write landed, so +// that number may be spent and never handed out. A caller tolerates gaps, never a number handed out twice. +// Deleting the key starts the count again at one; a key whose numbers must stay unique is never deleted. func (s KeptState) Increment(key string, attempts int) (uint64, error) { if attempts < 1 { return 0, fmt.Errorf("an increment of %s is tried at least once: %d attempts", key, attempts) @@ -96,6 +99,9 @@ func (s KeptState) Increment(key string, attempts int) (uint64, error) { _, err = s.Create(key, next) } else { var n uint64 + if string(e.Value) == "null" { + return 0, fmt.Errorf("%s holds null, not a counter", key) + } if err := json.Unmarshal(e.Value, &n); err != nil { return 0, fmt.Errorf("%s holds %s, not a counter: %w", key, e.Value, err) } diff --git a/go/state_counter_test.go b/go/state_counter_test.go index 6dfd220..764a1db 100644 --- a/go/state_counter_test.go +++ b/go/state_counter_test.go @@ -4,6 +4,7 @@ import ( "bufio" "encoding/json" "errors" + "fmt" "io" "strings" "sync" @@ -20,6 +21,7 @@ type casRuntime struct { revision uint64 keys map[string]StateEntry writes map[string]int + lost int refuse func(verb string) (code int, message string, refused bool) } @@ -34,6 +36,7 @@ func (r *casRuntime) answer(m message) map[string]any { defer r.mu.Unlock() verb := strings.TrimPrefix(m.Method, "mesh/state.") lost := func() map[string]any { + r.lost++ return map[string]any{"error": map[string]any{"code": CodeChanged, "message": p.Key + ": the key was written by another writer first"}} } if verb != "get" && r.refuse != nil { @@ -184,6 +187,10 @@ func TestTwoWritersIncrementingOneCounterLoseNoUpdate(t *testing.T) { if final != 2*each { t.Fatalf("the counter ended at %d, not %d: an increment was lost", final, 2*each) } + if r.lost == 0 { + t.Fatal("the two writers never collided: the test proved nothing about contention") + } + t.Logf("2000 increments, %d writes lost to the other writer and tried again", r.lost) if r.writes["create"] != 1 || r.writes["update"] != 2*each-1 || r.writes["put"] != 0 { t.Fatalf("written by %v: one create, then updates at the revision read, never a put", r.writes) } @@ -199,12 +206,9 @@ func TestAnIncrementThatLosesIsTriedAgainAndNeverReportedCounted(t *testing.T) { r.refuse = func(string) (int, string, bool) { if lost < 2 { lost++ - // Another writer was first: the counter moved on by one. + // Another writer was first: the counter moved on by one, to 42 and then 43. r.revision++ - r.keys["issue"] = StateEntry{Key: "issue", Value: json.RawMessage(`42`), Revision: r.revision} - if lost == 2 { - r.keys["issue"] = StateEntry{Key: "issue", Value: json.RawMessage(`43`), Revision: r.revision} - } + r.keys["issue"] = StateEntry{Key: "issue", Value: json.RawMessage(fmt.Sprint(41 + lost)), Revision: r.revision} return CodeChanged, "issue: the key was written by another writer first", true } return 0, "", false @@ -256,6 +260,14 @@ func TestAnIncrementThatLosesIsTriedAgainAndNeverReportedCounted(t *testing.T) { t.Fatalf("a value that is no counter was counted: %v, writes %v", got, r.writes) } }) + t.Run("null", func(t *testing.T) { + r := &casRuntime{keys: map[string]StateEntry{"issue": {Key: "issue", Value: json.RawMessage(`null`), Revision: 1}}, + writes: map[string]int{}, revision: 1} + got := counting(t, r, func() (any, error) { return State("numbers").Increment("issue", 5) }) + if _, isError := text(t, got); !isError || len(r.writes) != 0 { + t.Fatalf("a null was counted from zero: %v, writes %v", got, r.writes) + } + }) t.Run("no attempts", func(t *testing.T) { r := &casRuntime{keys: map[string]StateEntry{}, writes: map[string]int{}} got := counting(t, r, func() (any, error) { return State("numbers").Increment("issue", 0) })