Skip to content

Commit 15ab1a2

Browse files
juligasajuligasa
committed
fix(syncing): use WithTx for index writes (WithSave is now read-only)
main split reader/writer pool connections: Pool.WithSave now acquires a read-only connection and Pool.WithTx the writer. The maintained-index write paths (loadStore materialize, shadow-verify sweep) and their test fixtures used WithSave for INSERT/UPDATE/DELETE, landing on a read-only connection -> SQLITE_READONLY (the CI failure). Switch all main-DB writes to WithTx; reads and TEMP-only bodies (loadRBSRStore, collectBlobs) stay on WithSave. Tests use the file-backed MakeTestDB to avoid in-memory shared-cache schema locking. Reproduced the failure locally after rebasing onto main, then verified the fix passes with -race.
1 parent bf0b85f commit 15ab1a2

6 files changed

Lines changed: 17 additions & 30 deletions

File tree

‎backend/hmnet/syncing/canonical_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"seed/backend/hmnet/syncing/rbsr"
8+
"seed/backend/storage"
89
"seed/backend/util/sqlite"
910
"seed/backend/util/sqlite/sqlitex"
1011

@@ -39,8 +40,8 @@ func TestCanonicalization_MakesCodecsAgreeAcrossPeers(t *testing.T) {
3940
ctx := context.Background()
4041

4142
buildFingerprint := func(storedCodec int64, protocolVersion string) rbsr.Fingerprint {
42-
db := newMemDB(t)
43-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
43+
db := storage.MakeTestDB(t)
44+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
4445
if err := sqlitex.Exec(conn, `INSERT INTO rbsr_scope (id, iri, protocol_version, materialized) VALUES (1, 'hm://x', '0.9.2', 1)`, nil); err != nil {
4546
return err
4647
}

‎backend/hmnet/syncing/oracle_test.go‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,6 @@ func testSpace(t *testing.T) string {
2020
return coretest.NewTester("alice").Account.Principal().String()
2121
}
2222

23-
// newMemDB opens a single-connection in-memory database for tests. The shared
24-
// MakeTestDB/MakeTestMemoryDB helpers open a NumCPU-sized connection pool, which
25-
// under this package's parallel suite intermittently returns SQLITE_READONLY on
26-
// the first write in CI. A single-connection in-memory pool (the same config
27-
// TestDBQueries uses) is stable.
28-
func newMemDB(t *testing.T) *sqlitex.Pool {
29-
t.Helper()
30-
db, err := storage.OpenSQLite("file::memory:?mode=memory", 0, 1)
31-
require.NoError(t, err)
32-
t.Cleanup(func() { require.NoError(t, db.Close()) })
33-
require.NoError(t, storage.InitSQLiteSchema(db))
34-
return db
35-
}
36-
3723
func TestScopeCovers(t *testing.T) {
3824
t.Parallel()
3925

@@ -76,10 +62,10 @@ func TestScopeAllowsType(t *testing.T) {
7662
// base is "hm://<principal>".
7763
func oracleFixture(t *testing.T) (*sqlitex.Pool, string) {
7864
t.Helper()
79-
db := newMemDB(t)
65+
db := storage.MakeTestDB(t)
8066
base := "hm://" + testSpace(t)
8167

82-
require.NoError(t, db.WithSave(context.Background(), func(conn *sqlite.Conn) error {
68+
require.NoError(t, db.WithTx(context.Background(), func(conn *sqlite.Conn) error {
8369
for id, iri := range map[int]string{100: base, 101: base + "/doc", 102: base + "/doc/sub"} {
8470
if err := sqlitex.Exec(conn, `INSERT INTO resources (id, iri) VALUES (?, ?)`, nil, id, iri); err != nil {
8571
return err
@@ -228,9 +214,9 @@ func TestOracle_RefSeedsItsChangeClosure(t *testing.T) {
228214
// answer.
229215
func TestOracle_UncoveredEdgesReportIncomplete(t *testing.T) {
230216
t.Parallel()
231-
db := newMemDB(t)
217+
db := storage.MakeTestDB(t)
232218

233-
require.NoError(t, db.WithSave(context.Background(), func(conn *sqlite.Conn) error {
219+
require.NoError(t, db.WithTx(context.Background(), func(conn *sqlite.Conn) error {
234220
stmts := []string{
235221
`INSERT INTO resources (id, iri) VALUES (100, 'hm://s')`,
236222
`INSERT INTO resources (id, iri) VALUES (101, 'hm://s/doc')`,

‎backend/hmnet/syncing/rbsr_index_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestRbsrIndex_BuildMatchesLoadRBSRStore(t *testing.T) {
3939
require.NoError(t, want.Seal())
4040

4141
got := newAuthorizedTreeStore()
42-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
42+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
4343
id, materialized, err := resolveScope(conn, dkey, ProtocolVersionLegacy)
4444
if err != nil {
4545
return err
@@ -79,7 +79,7 @@ func TestRbsrIndex_IncrementalMaintenanceMatchesCollectBlobs(t *testing.T) {
7979
ctx := context.Background()
8080

8181
var scopeID int64
82-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
82+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
8383
id, _, err := resolveScope(conn, dkey, ProtocolVersionLegacy)
8484
if err != nil {
8585
return err
@@ -89,7 +89,7 @@ func TestRbsrIndex_IncrementalMaintenanceMatchesCollectBlobs(t *testing.T) {
8989
}))
9090

9191
// Index a new document under the scope: a Ref heading a new Change.
92-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
92+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
9393
stmts := []string{
9494
`INSERT INTO resources (id, iri) VALUES (200, '` + base + `/newdoc')`,
9595
`INSERT INTO blobs (id, multihash, codec, size) VALUES (70, X'70', 113, 1)`,
@@ -120,7 +120,7 @@ func TestRbsrIndex_ResolveScopeIdempotent(t *testing.T) {
120120
dkey := DiscoveryKey{IRI: blob.IRI(base), Recursive: true}
121121
ctx := context.Background()
122122

123-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
123+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
124124
id1, mat1, err := resolveScope(conn, dkey, ProtocolVersionLegacy)
125125
require.NoError(t, err)
126126
require.False(t, mat1)

‎backend/hmnet/syncing/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ func (s *Server) loadStoreLegacy(ctx context.Context, dkeys colx.HashSet[Discove
424424
// keeps the set current so reconciliation no longer rebuilds it per round.
425425
func (s *Server) loadStoreFromIndex(ctx context.Context, dkeys colx.HashSet[DiscoveryKey], authorizedSpaces []core.Principal, protocolVersion string) (rbsr.Store, error) {
426426
store := newAuthorizedTreeStore()
427-
if err := s.db.WithSave(ctx, func(conn *sqlite.Conn) error {
427+
if err := s.db.WithTx(ctx, func(conn *sqlite.Conn) error {
428428
scopeIDs := make([]int64, 0, len(dkeys))
429429
for dkey := range dkeys {
430430
id, materialized, err := resolveScope(conn, dkey, protocolVersion)

‎backend/hmnet/syncing/shadow_verify.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func ShadowVerifySweep(ctx context.Context, db *sqlitex.Pool) (checked, drifted
121121

122122
for _, s := range scopes {
123123
var ok bool
124-
if err := db.WithSave(ctx, func(conn *sqlite.Conn) error {
124+
if err := db.WithTx(ctx, func(conn *sqlite.Conn) error {
125125
var e error
126126
ok, e = shadowVerifyScope(conn, s.id, s.dkey)
127127
return e

‎backend/hmnet/syncing/shadow_verify_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func TestShadowVerify_DetectsDriftAndMarksStale(t *testing.T) {
2121
ctx := context.Background()
2222

2323
var scopeID int64
24-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
24+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
2525
id, _, err := resolveScope(conn, dkey, ProtocolVersionLegacy)
2626
if err != nil {
2727
return err
@@ -31,7 +31,7 @@ func TestShadowVerify_DetectsDriftAndMarksStale(t *testing.T) {
3131
}))
3232

3333
// A faithfully materialized scope must verify clean and stay materialized.
34-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
34+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
3535
ok, err := shadowVerifyScope(conn, scopeID, dkey)
3636
require.NoError(t, err)
3737
require.True(t, ok, "faithful set must verify clean")
@@ -40,11 +40,11 @@ func TestShadowVerify_DetectsDriftAndMarksStale(t *testing.T) {
4040
require.True(t, scopeMaterialized(t, db, scopeID), "clean scope stays materialized")
4141

4242
// Corrupt the maintained set by dropping a blob it should contain.
43-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
43+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
4444
return sqlitex.Exec(conn, `DELETE FROM rbsr_item WHERE scope = ? AND blob = 11`, nil, scopeID)
4545
}))
4646

47-
require.NoError(t, db.WithSave(ctx, func(conn *sqlite.Conn) error {
47+
require.NoError(t, db.WithTx(ctx, func(conn *sqlite.Conn) error {
4848
ok, err := shadowVerifyScope(conn, scopeID, dkey)
4949
require.NoError(t, err)
5050
require.False(t, ok, "drift must be detected")

0 commit comments

Comments
 (0)