mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-08 22:22:52 +00:00
chore(lbug): isDbBusyError review fixes
- Drop redundant `could not set lock` term — already subsumed by `lock`.
- Document the intentionally-broad matcher: graph-DB lock-shaped errors
("deadlock", "unlock failed", "lock contention", "could not open lock
file") are all treated as transient. If a non-transient surfaces,
tighten the matcher rather than raise the retry budget.
- Add positive test cases covering those lock-shaped strings so the
intent is visible and a future tightening would deliberately break
these.
- Fix the open-retry back-off comment: max sleep is 100+200+300+400 =
1000ms (no sleep after the final attempt), not 1.5s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c4782e2db9
commit
6ffef3ab99
2 changed files with 22 additions and 8 deletions
|
|
@ -82,12 +82,13 @@ export interface LbugConnectionHandle {
|
|||
*/
|
||||
export const isDbBusyError = (err: unknown): boolean => {
|
||||
const msg = (err instanceof Error ? err.message : String(err)).toLowerCase();
|
||||
return (
|
||||
msg.includes('busy') ||
|
||||
msg.includes('lock') ||
|
||||
msg.includes('already in use') ||
|
||||
msg.includes('could not set lock')
|
||||
);
|
||||
// `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');
|
||||
};
|
||||
|
||||
export function createLbugDatabase(
|
||||
|
|
@ -126,8 +127,9 @@ export function createLbugDatabase(
|
|||
// OS-level exclusive lock on `<dbPath>`. On Windows that lock can fail
|
||||
// for reasons specific to the OS (Defender briefly opens new files,
|
||||
// libuv handle release lags the JS-side close). 5 attempts × 100ms
|
||||
// linear back-off (~1.5s worst case) clears the typical AV-scanner hold
|
||||
// without masking real cross-process conflicts.
|
||||
// linear back-off (max sleep 100+200+300+400 = 1s, plus 5 ctor RTTs
|
||||
// of 10–50ms each = ~1.0–1.2s worst case) clears the typical
|
||||
// AV-scanner hold without masking real cross-process conflicts.
|
||||
//
|
||||
// Source: https://github.com/LadybugDB/ladybug/blob/v0.16.1/src/common/file_system/local_file_system.cpp#L126
|
||||
const OPEN_LOCK_RETRY_ATTEMPTS = 5;
|
||||
|
|
|
|||
|
|
@ -46,6 +46,18 @@ describe('isDbBusyError', () => {
|
|||
expect(isDbBusyError(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
// Documented behavior for lock-shaped strings: the matcher is intentionally
|
||||
// broad because in graph-DB contexts these are all transient. If LadybugDB
|
||||
// ever surfaces a non-transient lock-shaped error (e.g., a recovery-time
|
||||
// "lock file missing"), tighten the matcher and add a negative test here
|
||||
// rather than raising the retry budget.
|
||||
it('treats other lock-shaped errors as transient (current intentional behavior)', () => {
|
||||
expect(isDbBusyError(new Error('deadlock detected'))).toBe(true);
|
||||
expect(isDbBusyError(new Error('unlock failed'))).toBe(true);
|
||||
expect(isDbBusyError(new Error('lock contention'))).toBe(true);
|
||||
expect(isDbBusyError(new Error('Could not open lock file'))).toBe(true);
|
||||
});
|
||||
|
||||
it('handles non-Error values gracefully', () => {
|
||||
expect(isDbBusyError('BUSY error')).toBe(true);
|
||||
expect(isDbBusyError(42)).toBe(false);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue