git.iko.soy/crush
on main help sign in
fix(db): only enforce the data directory lock in client server mode

The exclusive lock on a data directory was previously taken on every Crush startup, which broke the long standing local mode workflow of running two Crush instances in the same directory. The lock is now opt in via a new option, and only the shared server takes it. Local mode keeps its previous behavior, and the existing environment variable escape hatch continues to work. Co-Authored-By: Charm Crush <crush@charm.land>

commitc986a35c465fdc22fbaede2609522e3e1997e632 authorChristian Rocha <christian@rocha.is> dateWed 20 May 2026, 07:15 (5 months ago) parentf9676eb
3 files changed+92−17
compare with main
unifiedsplit
internal/backend/backend.go+1−1 internal/db/connect.go+45−9 internal/db/connect_test.go+46−7
internal/backend/backend.go+1−1
@@ -223,7 +223,7 @@ func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Works
223223 return nil, proto.Workspace{}, fmt.Errorf("failed to create data directory: %w", err)
224224 }
225225
226− conn, err := db.Connect(b.ctx, cfg.Config().Options.DataDirectory)
226+ conn, err := db.Connect(b.ctx, cfg.Config().Options.DataDirectory, db.WithDataDirLock(true))
227227 if err != nil {
228228 return nil, proto.Workspace{}, fmt.Errorf("failed to connect to database: %w", err)
229229 }
internal/db/connect.go+45−9
@@ -56,16 +56,39 @@ var (
5656 poolMu sync.Mutex
5757)
5858
59+// ConnectOption configures a Connect call. Options are applied in
60+// order; later options override earlier ones for the same field.
61+type ConnectOption func(*connectOptions)
62+
63+// connectOptions holds the resolved configuration for a Connect call.
64+type connectOptions struct {
65+ lockDataDir bool
66+}
67+
68+// WithDataDirLock toggles acquisition of the per-data-directory lock
69+// for this Connect call. The lock is off by default so local-mode
70+// invocations do not regress today's behavior; the server's
71+// workspace-bootstrap path opts in. CRUSH_SKIP_DATADIR_LOCK still
72+// bypasses acquisition even when this option is set.
73+func WithDataDirLock(enable bool) ConnectOption {
74+ return func(o *connectOptions) { o.lockDataDir = enable }
75+}
76+
5977// Connect opens a SQLite database connection for the given data
6078// directory and runs migrations. If a connection to the same database
6179// file already exists, the existing connection is returned with its
6280// reference count incremented. Callers must pair each Connect with a
6381// [Release] when they no longer need the connection.
64−func Connect(ctx context.Context, dataDir string) (*sql.DB, error) {
82+func Connect(ctx context.Context, dataDir string, opts ...ConnectOption) (*sql.DB, error) {
6583 if dataDir == "" {
6684 return nil, fmt.Errorf("data.dir is not set")
6785 }
6886
87+ var cfg connectOptions
88+ for _, opt := range opts {
89+ opt(&cfg)
90+ }
91+
6992 dbPath := filepath.Join(dataDir, "crush.db")
7093
7194 // Resolve to an absolute path so that different relative paths to
@@ -88,18 +111,25 @@ func Connect(ctx context.Context, dataDir string) (*sql.DB, error) {
88111 // crush process on the same SQLite file. The lock is released when
89112 // the matching Release call drops the refcount to zero. Ensuring
90113 // the data directory exists is required because the lock file
91− // lives inside it.
114+ // lives inside it. Locking is opt-in via WithDataDirLock so that
115+ // local-mode invocations do not refuse a second crush against the
116+ // same data dir until client/server becomes the default.
92117 if err := os.MkdirAll(dataDir, 0o700); err != nil {
93118 return nil, fmt.Errorf("failed to create data directory %q: %w", dataDir, err)
94119 }
95− lock, err := acquireDataDirLock(dataDir)
96− if err != nil {
97− return nil, err
120+ var lock *dataDirLock
121+ if cfg.lockDataDir && !skipDataDirLock() {
122+ lock, err = acquireDataDirLock(dataDir)
123+ if err != nil {
124+ return nil, err
125+ }
98126 }
99127
100128 conn, err := openDB(dbPath)
101129 if err != nil {
102− lock.release()
130+ if lock != nil {
131+ lock.release()
132+ }
103133 return nil, err
104134 }
105135
@@ -110,22 +140,28 @@ func Connect(ctx context.Context, dataDir string) (*sql.DB, error) {
110140 // resulting in SQLITE_NOTADB (26) on the next open.
111141 conn.SetMaxOpenConns(1)
112142
143+ releaseLock := func() {
144+ if lock != nil {
145+ lock.release()
146+ }
147+ }
148+
113149 if err = conn.PingContext(ctx); err != nil {
114150 conn.Close()
115− lock.release()
151+ releaseLock()
116152 return nil, fmt.Errorf("failed to connect to database: %w", err)
117153 }
118154
119155 if err := initGoose(); err != nil {
120156 conn.Close()
121− lock.release()
157+ releaseLock()
122158 slog.Error("Failed to initialize goose", "error", err)
123159 return nil, fmt.Errorf("failed to initialize goose: %w", err)
124160 }
125161
126162 if err := goose.Up(conn, "migrations"); err != nil {
127163 conn.Close()
128− lock.release()
164+ releaseLock()
129165 slog.Error("Failed to apply migrations", "error", err)
130166 return nil, fmt.Errorf("failed to apply migrations: %w", err)
131167 }
internal/db/connect_test.go+46−7
@@ -69,7 +69,7 @@ func TestConnect_FailsWhenDataDirLocked(t *testing.T) {
6969 require.NoError(t, err, "expected to take the data-dir lock for the first time")
7070 t.Cleanup(release)
7171
72− _, err = Connect(context.Background(), dataDir)
72+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
7373 require.Error(t, err, "Connect must refuse to open a locked data dir")
7474 require.ErrorIs(t, err, ErrDataDirLocked)
7575}
@@ -85,12 +85,12 @@ func TestConnect_SucceedsAfterContenderReleases(t *testing.T) {
8585 release, err := tryFileLock(lockPath)
8686 require.NoError(t, err)
8787
88− _, err = Connect(context.Background(), dataDir)
88+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
8989 require.ErrorIs(t, err, ErrDataDirLocked)
9090
9191 release()
9292
93− conn, err := Connect(context.Background(), dataDir)
93+ conn, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
9494 require.NoError(t, err, "Connect should succeed once the contender releases the lock")
9595 require.NoError(t, conn.PingContext(context.Background()))
9696 require.NoError(t, Release(dataDir))
@@ -105,7 +105,7 @@ func TestConnect_LockReleasedOnFinalRelease(t *testing.T) {
105105 dataDir := t.TempDir()
106106 lockPath := filepath.Join(dataDir, dataDirLockFile)
107107
108− conn, err := Connect(context.Background(), dataDir)
108+ conn, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
109109 require.NoError(t, err)
110110 require.NoError(t, conn.PingContext(context.Background()))
111111
@@ -135,10 +135,10 @@ func TestConnect_SharedPoolDoesNotReacquireLock(t *testing.T) {
135135 dataDir := t.TempDir()
136136 lockPath := filepath.Join(dataDir, dataDirLockFile)
137137
138− _, err := Connect(context.Background(), dataDir)
138+ _, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
139139 require.NoError(t, err)
140140
141− _, err = Connect(context.Background(), dataDir)
141+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
142142 require.NoError(t, err)
143143
144144 // Drop one reference; lock must still be held.
@@ -163,8 +163,47 @@ func TestConnect_SkipLockEnvBypassesAcquisition(t *testing.T) {
163163
164164 t.Setenv("CRUSH_SKIP_DATADIR_LOCK", "1")
165165
166− conn, err := Connect(context.Background(), dataDir)
166+ conn, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
167167 require.NoError(t, err, "skip-lock env should bypass contention")
168168 require.NoError(t, conn.PingContext(context.Background()))
169169 require.NoError(t, Release(dataDir))
170170}
171+
172+// TestConnect_DefaultIgnoresContendedLock confirms that without
173+// WithDataDirLock(true) the lock file is irrelevant: a contender can
174+// hold tryFileLock and Connect still succeeds. This pins the
175+// local-mode default to its pre-lock behavior.
176+func TestConnect_DefaultIgnoresContendedLock(t *testing.T) {
177+ t.Cleanup(ResetPool)
178+
179+ dataDir := t.TempDir()
180+ lockPath := filepath.Join(dataDir, dataDirLockFile)
181+
182+ release, err := tryFileLock(lockPath)
183+ require.NoError(t, err, "expected to take the data-dir lock for the first time")
184+ t.Cleanup(release)
185+
186+ conn, err := Connect(context.Background(), dataDir)
187+ require.NoError(t, err, "default Connect must not take the lock and must succeed under contention")
188+ require.NoError(t, conn.PingContext(context.Background()))
189+ require.NoError(t, Release(dataDir))
190+}
191+
192+// TestConnect_ServerPathFailsWhenDataDirLocked is the server's
193+// workspace-bootstrap analogue of TestConnect_FailsWhenDataDirLocked:
194+// passing WithDataDirLock(true) must surface ErrDataDirLocked when a
195+// contender already holds the lock.
196+func TestConnect_ServerPathFailsWhenDataDirLocked(t *testing.T) {
197+ t.Cleanup(ResetPool)
198+
199+ dataDir := t.TempDir()
200+ lockPath := filepath.Join(dataDir, dataDirLockFile)
201+
202+ release, err := tryFileLock(lockPath)
203+ require.NoError(t, err, "expected to take the data-dir lock for the first time")
204+ t.Cleanup(release)
205+
206+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
207+ require.Error(t, err, "server-path Connect must refuse to open a locked data dir")
208+ require.ErrorIs(t, err, ErrDataDirLocked)
209+}