mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-09 03:17:54 +00:00
fix(lbug): retry single-writer transaction contention (#2342)
This commit is contained in:
parent
859e4b75a4
commit
365de846d1
4 changed files with 108 additions and 12 deletions
|
|
@ -61,7 +61,7 @@ Format: **Trigger → Instruction → Reason**. Append new Signs when the same m
|
|||
|
||||
- **Trigger:** Errors opening `.gitnexus/lbug` while MCP and analyze both run.
|
||||
- **Do:** Stop overlapping processes (one writer at a time). Retry analyze or restart MCP.
|
||||
- **Why:** Embedded DB expects single-process ownership. Known gap: as of `@ladybugdb/core` 0.18.0, one contention error — `"Only one write transaction at a time is allowed in the system."` — isn't recognized by our busy/lock retry matcher (`isDbBusyError` in `src/core/lbug/lbug-config.ts`), so it surfaces as a raw failure instead of a retried one. If you see that exact message, it's the same "one writer at a time" issue above, not a new failure mode.
|
||||
- **Why:** Embedded DB expects single-process ownership. `@ladybugdb/core` 0.18.0 also reports this contention as `"Only one write transaction at a time is allowed in the system."` — our busy/lock retry matcher (`isDbBusyError` in `src/core/lbug/lbug-config.ts`) recognizes this exact string too, so it's auto-retried the same as any other lock error. If you see that exact message, it's the same "one writer at a time" issue above, not a new failure mode.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -154,7 +154,7 @@ Analyze re-execs Node with a **large old-space heap** when needed (`analyze.ts`)
|
|||
|
||||
Only one process should open a repo's `.gitnexus/lbug` store at a time. If MCP and a second `analyze` run conflict, stop one process, then retry `analyze` or restart MCP.
|
||||
|
||||
If the error text is `"Only one write transaction at a time is allowed in the system."` instead of a lock/busy message, it's the same underlying conflict — our retry matcher doesn't currently recognize that exact string (see `isDbBusyError` in `src/core/lbug/lbug-config.ts`), so it isn't auto-retried. The fix is the same: stop the overlapping process.
|
||||
If the error text is `"Only one write transaction at a time is allowed in the system."` instead of a lock/busy message, it's the same underlying conflict — our retry matcher (`isDbBusyError` in `src/core/lbug/lbug-config.ts`) recognizes this exact string and auto-retries it. The fix if it still surfaces after retries is the same: stop the overlapping process.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -368,10 +368,13 @@ export interface LbugConnectionHandle {
|
|||
}
|
||||
|
||||
/**
|
||||
* Return true when the error message indicates that a LadybugDB file lock
|
||||
* could not be acquired — either at construction time
|
||||
* (`new lbug.Database(...)` raises from `local_file_system.cpp`) or during
|
||||
* a query (another writer holds the exclusive lock).
|
||||
* Return true when the error message indicates that a LadybugDB write
|
||||
* transaction could not proceed due to lock contention — either a file
|
||||
* lock that could not be acquired (either at construction time,
|
||||
* `new lbug.Database(...)` raising from `local_file_system.cpp`, or during
|
||||
* a query, another writer holds the exclusive lock), or a same-process
|
||||
* write transaction rejected because another write transaction is already
|
||||
* active on the connection.
|
||||
*
|
||||
* Lives here (not in `lbug-adapter.ts`) so both the construction-time
|
||||
* retry (`openWithLockRetry` in this file) and the query-time retry
|
||||
|
|
@ -383,10 +386,21 @@ export const isDbBusyError = (err: unknown): boolean => {
|
|||
// `lock` already subsumes `could not set lock`; the broader term is kept
|
||||
// because graph-DB transient errors include "deadlock", "lock contention",
|
||||
// and the LadybugDB native module's "could not set lock on file" — all of
|
||||
// which deserve a retry. If a non-transient lock-shaped error ever
|
||||
// surfaces (e.g., "lock file missing" during recovery), tighten this
|
||||
// matcher rather than raising the retry budget.
|
||||
return msg.includes('busy') || msg.includes('lock') || msg.includes('already in use');
|
||||
// which deserve a retry. LadybugDB also reports same-process writer
|
||||
// contention without the words "busy" or "lock".
|
||||
//
|
||||
// "only one write transaction at a time" was observed against LadybugDB
|
||||
// 0.18.0 (see gitnexus/package.json @ladybugdb/core).
|
||||
//
|
||||
// If a non-transient lock-shaped error ever surfaces (e.g., "lock file
|
||||
// missing" during recovery), tighten this matcher rather than raising the
|
||||
// retry budget.
|
||||
return (
|
||||
msg.includes('busy') ||
|
||||
msg.includes('lock') ||
|
||||
msg.includes('already in use') ||
|
||||
msg.includes('only one write transaction at a time')
|
||||
);
|
||||
};
|
||||
|
||||
export function createLbugDatabase(
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import { withTestLbugDB } from '../helpers/test-indexed-db.js';
|
|||
|
||||
// Pure-function tests — no DB needed, but grouped here for cohesion
|
||||
// with the retry logic they guard.
|
||||
import { isDbBusyError } from '../../src/core/lbug/lbug-config.js';
|
||||
import { isDbBusyError, openLbugConnection } from '../../src/core/lbug/lbug-config.js';
|
||||
|
||||
describe('isDbBusyError', () => {
|
||||
it('returns true for "busy" errors (case-insensitive)', () => {
|
||||
|
|
@ -34,6 +34,15 @@ describe('isDbBusyError', () => {
|
|||
expect(isDbBusyError('already in use')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true for "only one write transaction at a time" errors', () => {
|
||||
expect(
|
||||
isDbBusyError(new Error('Only one write transaction at a time is allowed in the system.')),
|
||||
).toBe(true);
|
||||
expect(isDbBusyError('only one write transaction at a time is allowed in the system.')).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it('returns true for "could not set lock" errors', () => {
|
||||
expect(isDbBusyError(new Error('Could not set lock on the database file'))).toBe(true);
|
||||
});
|
||||
|
|
@ -65,6 +74,48 @@ describe('isDbBusyError', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// ─── openLbugConnection construction-time retry ────────────────────────────
|
||||
|
||||
// Minimal stub of the `lbug` module surface used by openLbugConnection.
|
||||
// Duplicated locally (see lbug-open-retry.test.ts's makeStubLbug) rather
|
||||
// than shared, matching this codebase's existing per-test-file convention.
|
||||
interface StubModuleControl {
|
||||
databaseThrows: Array<Error | null>;
|
||||
databaseCallCount: number;
|
||||
}
|
||||
|
||||
const makeStubLbug = (control: StubModuleControl) => {
|
||||
class FakeDatabase {
|
||||
constructor(_path: string, ..._rest: unknown[]) {
|
||||
control.databaseCallCount++;
|
||||
const next = control.databaseThrows.shift();
|
||||
if (next instanceof Error) throw next;
|
||||
}
|
||||
async close(): Promise<void> {}
|
||||
}
|
||||
class FakeConnection {
|
||||
constructor(_db: FakeDatabase) {}
|
||||
async close(): Promise<void> {}
|
||||
}
|
||||
return { Database: FakeDatabase, Connection: FakeConnection } as any;
|
||||
};
|
||||
|
||||
describe('openLbugConnection — write-transaction contention retry', () => {
|
||||
it('retries on write-transaction contention and succeeds on a later attempt', async () => {
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [
|
||||
new Error('Only one write transaction at a time is allowed in the system.'),
|
||||
null,
|
||||
],
|
||||
databaseCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
const handle = await openLbugConnection(stub, '/some/path/lbug');
|
||||
expect(handle.db).toBeDefined();
|
||||
expect(control.databaseCallCount).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── withLbugDb retry integration tests ───────────────────────────────────
|
||||
|
||||
withTestLbugDB('lock-retry', (handle) => {
|
||||
|
|
@ -88,6 +139,21 @@ withTestLbugDB('lock-retry', (handle) => {
|
|||
expect(callCount).toBe(2);
|
||||
});
|
||||
|
||||
it('retries on LadybugDB single-writer transaction contention', async () => {
|
||||
const { withLbugDb } = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
let callCount = 0;
|
||||
const result = await withLbugDb(handle.dbPath, async () => {
|
||||
callCount++;
|
||||
if (callCount === 1) {
|
||||
throw new Error('Only one write transaction at a time is allowed in the system.');
|
||||
}
|
||||
return 'recovered';
|
||||
});
|
||||
|
||||
expect(result).toBe('recovered');
|
||||
expect(callCount).toBe(2);
|
||||
});
|
||||
|
||||
it('propagates non-BUSY errors immediately without retrying', async () => {
|
||||
const { withLbugDb } = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
let callCount = 0;
|
||||
|
|
@ -111,7 +177,23 @@ withTestLbugDB('lock-retry', (handle) => {
|
|||
}),
|
||||
).rejects.toThrow('Could not set lock');
|
||||
|
||||
// DB_LOCK_RETRY_ATTEMPTS = 3 (default in the implementation)
|
||||
// Matches DB_LOCK_RETRY_ATTEMPTS in lbug-adapter.ts. If that budget
|
||||
// changes, this assertion — not this comment — is the source of truth.
|
||||
expect(callCount).toBe(3);
|
||||
});
|
||||
|
||||
it('throws after max retry attempts on write-transaction contention', async () => {
|
||||
const { withLbugDb } = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
let callCount = 0;
|
||||
await expect(
|
||||
withLbugDb(handle.dbPath, async () => {
|
||||
callCount++;
|
||||
throw new Error('Only one write transaction at a time is allowed in the system.');
|
||||
}),
|
||||
).rejects.toThrow('Only one write transaction at a time is allowed in the system.');
|
||||
|
||||
// Matches DB_LOCK_RETRY_ATTEMPTS in lbug-adapter.ts. If that budget
|
||||
// changes, this assertion — not this comment — is the source of truth.
|
||||
expect(callCount).toBe(3);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue