mirror of
https://github.com/tiennm99/tiennm99bot.git
synced 2026-10-14 12:17:10 +00:00
refactor(modules): extract per-subject lock to internal/keylock
Extracted per-subject RWMutex primitive into dedicated internal/keylock package to allow reuse across game modules. Wordle previously held local subjectLocks; loldle module (Phase 5c) would duplicate this type without the extraction. Placing keylock as a peer to storage/telegram (not nested under modules/) matches its cross-module scope.
This commit is contained in:
1 parent
8c62d73895
commit
676c481def
4 files changed
+116
-36
No files matched your search
@@ -0,0 +1,33 @@
|
||||
// Package keylock serialises compound operations that target the same key
|
||||
// (typically a chat / user / subject identifier) across goroutines.
|
||||
//
|
||||
// Why a separate package: every game module needs a per-subject mutex to
|
||||
// turn KVStore's single-op atomicity into safe Get→mutate→Put. The bot
|
||||
// dispatcher runs each Telegram update in its own goroutine, and the JS
|
||||
// source's Cloudflare Workers serialisation is not a property Go inherits.
|
||||
//
|
||||
// Trade-off: the underlying sync.Map grows unboundedly with distinct keys
|
||||
// (~32 B each). At 1M keys that's ~32 MB — acceptable for the lifetime of
|
||||
// a Cloud Run instance, which restarts well before reaching that scale.
|
||||
// Eviction is a Phase 11 concern, not a v1 one.
|
||||
package keylock
|
||||
|
||||
import "sync"
|
||||
|
||||
// Map gives each string key its own mutex, lazily created. Zero value is
|
||||
// usable; do not copy after first use (sync.Map is non-copyable).
|
||||
type Map struct {
|
||||
m sync.Map // key: string → val: *sync.Mutex
|
||||
}
|
||||
|
||||
// Acquire locks the per-key mutex and returns its Unlock as a func so the
|
||||
// caller can `defer m.Acquire(key)()` at the top of a critical section.
|
||||
//
|
||||
// Distinct keys never block each other; same-key callers run serially in the
|
||||
// order Acquire was called.
|
||||
func (m *Map) Acquire(key string) func() {
|
||||
v, _ := m.m.LoadOrStore(key, &sync.Mutex{})
|
||||
mu := v.(*sync.Mutex)
|
||||
mu.Lock()
|
||||
return mu.Unlock
|
||||
}
|
||||
@@ -0,0 +1,73 @@
|
||||
package keylock
|
||||
|
||||
import (
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestMap_DistinctKeysDoNotBlock asserts the per-key fan-out: two acquires on
|
||||
// different keys must run in parallel even while one is held. We arrange a
|
||||
// goroutine that holds key "a" for a beat, then a second goroutine acquires
|
||||
// "b" — if Acquire blocked across keys, the second goroutine couldn't finish
|
||||
// before the first releases. We give it a generous timeout to avoid flakes
|
||||
// on slow CI hardware while still failing if locks are global.
|
||||
func TestMap_DistinctKeysDoNotBlock(t *testing.T) {
|
||||
var m Map
|
||||
holdA := make(chan struct{})
|
||||
releasedA := make(chan struct{})
|
||||
bDone := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
release := m.Acquire("a")
|
||||
close(holdA)
|
||||
<-time.After(50 * time.Millisecond)
|
||||
release()
|
||||
close(releasedA)
|
||||
}()
|
||||
|
||||
go func() {
|
||||
<-holdA
|
||||
release := m.Acquire("b")
|
||||
release()
|
||||
close(bDone)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-bDone:
|
||||
// Good: b acquired while a was held.
|
||||
case <-time.After(40 * time.Millisecond):
|
||||
t.Fatal("Acquire(\"b\") blocked while Acquire(\"a\") was held — keys are not independent")
|
||||
}
|
||||
<-releasedA
|
||||
}
|
||||
|
||||
// TestMap_SameKeySerialises asserts mutual exclusion on a shared key under
|
||||
// concurrent contention. If acquires interleaved, the counter would race.
|
||||
func TestMap_SameKeySerialises(t *testing.T) {
|
||||
var m Map
|
||||
var counter int64
|
||||
const goroutines = 32
|
||||
const itersEach = 100
|
||||
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(goroutines)
|
||||
for i := 0; i < goroutines; i++ {
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
for j := 0; j < itersEach; j++ {
|
||||
release := m.Acquire("shared")
|
||||
// Read-modify-write that would race without serialisation.
|
||||
v := atomic.LoadInt64(&counter)
|
||||
atomic.StoreInt64(&counter, v+1)
|
||||
release()
|
||||
}
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
if got := atomic.LoadInt64(&counter); got != int64(goroutines*itersEach) {
|
||||
t.Errorf("counter = %d, want %d (lost updates → mutex didn't serialise)", got, goroutines*itersEach)
|
||||
}
|
||||
}
|
||||
@@ -3,6 +3,7 @@ package wordle
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"math"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -10,6 +11,7 @@ import (
|
||||
"github.com/go-telegram/bot"
|
||||
"github.com/go-telegram/bot/models"
|
||||
|
||||
"github.com/tiennm99/miti99bot-go/internal/keylock"
|
||||
"github.com/tiennm99/miti99bot-go/internal/storage"
|
||||
)
|
||||
|
||||
@@ -19,7 +21,7 @@ type state struct {
|
||||
kv storage.KVStore
|
||||
words []string
|
||||
set map[string]struct{}
|
||||
locks subjectLocks // per-subject mutex; serialises Get→mutate→Put
|
||||
locks keylock.Map // per-subject mutex; serialises Get→mutate→Put
|
||||
}
|
||||
|
||||
// subjectFor mirrors JS getSubject: per-user in DMs, per-chat in groups,
|
||||
@@ -125,7 +127,7 @@ func (s *state) handleWordle(ctx context.Context, b *bot.Bot, update *models.Upd
|
||||
if subject == "" {
|
||||
return reply(ctx, b, msg, "Cannot identify chat.")
|
||||
}
|
||||
defer s.locks.acquire(subject)()
|
||||
defer s.locks.Acquire(subject)()
|
||||
arg := argAfterCommand(msg.Text)
|
||||
|
||||
g, err := s.getOrInit(ctx, subject)
|
||||
@@ -197,7 +199,7 @@ func (s *state) handleNew(ctx context.Context, b *bot.Bot, update *models.Update
|
||||
if subject == "" {
|
||||
return reply(ctx, b, msg, "Cannot identify chat.")
|
||||
}
|
||||
defer s.locks.acquire(subject)()
|
||||
defer s.locks.Acquire(subject)()
|
||||
|
||||
prelude := ""
|
||||
prior, err := loadGame(ctx, s.kv, subject)
|
||||
@@ -229,7 +231,7 @@ func (s *state) handleGiveup(ctx context.Context, b *bot.Bot, update *models.Upd
|
||||
if subject == "" {
|
||||
return reply(ctx, b, msg, "Cannot identify chat.")
|
||||
}
|
||||
defer s.locks.acquire(subject)()
|
||||
defer s.locks.Acquire(subject)()
|
||||
g, err := s.getOrInit(ctx, subject)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -266,7 +268,10 @@ func (s *state) handleStats(ctx context.Context, b *bot.Bot, update *models.Upda
|
||||
}
|
||||
winRate := 0
|
||||
if stats.Played > 0 {
|
||||
winRate = int(float64(stats.Wins) / float64(stats.Played) * 100)
|
||||
// math.Round matches JS Math.round (round half away from zero for
|
||||
// positive inputs); int(...) would truncate 66.66 to 66 where JS
|
||||
// shows 67.
|
||||
winRate = int(math.Round(float64(stats.Wins) / float64(stats.Played) * 100))
|
||||
}
|
||||
scope := "group"
|
||||
if msg.Chat.Type == models.ChatTypePrivate {
|
||||
|
||||
@@ -1,31 +0,0 @@
|
||||
package wordle
|
||||
|
||||
import "sync"
|
||||
|
||||
// subjectLocks serialises Get → mutate → Put compound operations on a
|
||||
// GameState by per-subject mutex.
|
||||
//
|
||||
// Why: the KVStore guarantees atomicity on a single op (Get, Put, Delete) but
|
||||
// not on a load-then-save sequence. With the bot dispatcher running each
|
||||
// Telegram update in its own goroutine, two concurrent /wordle calls for the
|
||||
// same chat (groups can have many active users) can interleave and silently
|
||||
// drop one player's guess. The Cloudflare Workers source the JS bot ran in
|
||||
// happens to serialise this for free; Go + Firestore does not.
|
||||
//
|
||||
// Implementation: one *sync.Mutex per subject, lazily created via sync.Map.
|
||||
// The map grows unboundedly with distinct subjects but each entry is ~32 B,
|
||||
// so 1M chats costs ~32 MB — acceptable for v1. Sharded eviction is a Phase
|
||||
// 11 concern.
|
||||
type subjectLocks struct {
|
||||
m sync.Map // key: subject string → val: *sync.Mutex
|
||||
}
|
||||
|
||||
// acquire locks the per-subject mutex and returns the unlock func. Callers
|
||||
// must `defer s.acquire(subject)()` at the top of any handler that reads,
|
||||
// mutates, then writes the same KV record.
|
||||
func (s *subjectLocks) acquire(subject string) func() {
|
||||
v, _ := s.m.LoadOrStore(subject, &sync.Mutex{})
|
||||
mu := v.(*sync.Mutex)
|
||||
mu.Lock()
|
||||
return mu.Unlock
|
||||
}
|
||||
Reference in new issue
Block a user