Open each event database once when callers race (closes #291)
check / check (push) Successful in 3m48s
check / check (push) Successful in 3m48s
GetDB opened the database on a cache miss and then tried to cache it, so callers racing on a webhook's first use could each open the file, and the losers closed their copies. On a new file the parallel opens also create its tables at the same time, and one caller can fail with "table already exists". A mutex now covers the open: GetDB looks in the cache again under it, then opens and caches. DeleteDB and CloseAll take the same mutex, so neither runs while an open is under way. Reading an already cached database takes no lock. The new test starts many callers on one webhook at once and checks that exactly one open happened. Model: opus-5-5
This commit was merged in pull request #327.
This commit is contained in:
@@ -1,10 +1,14 @@
|
||||
package database_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
@@ -104,6 +108,54 @@ func TestWebhookDBManager_CreateAndGetDB(t *testing.T) {
|
||||
assert.Equal(t, `{"test": true}`, readEvent.Body)
|
||||
}
|
||||
|
||||
// Many callers ask for one webhook's database at the same moment,
|
||||
// before it is cached. Only one of them may open the file; the others
|
||||
// must wait for its handle. openDB logs one "opened per-webhook
|
||||
// database" line per open, and those lines are what is counted.
|
||||
func TestWebhookDBManager_ConcurrentFirstTouchOpensOnce(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var logs bytes.Buffer
|
||||
|
||||
mgr := database.NewTestWebhookDBManagerWithLogger(
|
||||
t.TempDir(),
|
||||
slog.New(slog.NewTextHandler(&logs, nil)),
|
||||
)
|
||||
|
||||
t.Cleanup(func() { assert.NoError(t, mgr.CloseAll()) })
|
||||
|
||||
webhookID := uuid.New().String()
|
||||
|
||||
const callers = 16
|
||||
|
||||
start := make(chan struct{})
|
||||
handles := make([]*gorm.DB, callers)
|
||||
errs := make([]error, callers)
|
||||
|
||||
var wg sync.WaitGroup
|
||||
|
||||
for i := range callers {
|
||||
wg.Go(func() {
|
||||
<-start
|
||||
|
||||
handles[i], errs[i] = mgr.GetDB(webhookID)
|
||||
})
|
||||
}
|
||||
|
||||
close(start)
|
||||
wg.Wait()
|
||||
|
||||
for i := range callers {
|
||||
require.NoError(t, errs[i])
|
||||
assert.Same(t, handles[0], handles[i])
|
||||
}
|
||||
|
||||
assert.Equal(
|
||||
t, 1,
|
||||
strings.Count(logs.String(), "opened per-webhook database"),
|
||||
)
|
||||
}
|
||||
|
||||
func TestWebhookDBManager_DeleteDB(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user