Skip to content

Commit 85ce4ff

Browse files
authored
fix: respect retry options in acquireBatch method (#63)
* fix: respect retry options in acquireBatch method - Add retryAttempts and retryDelay options to acquireBatch and usingBatch - Implement retry loop matching SimpleLock.acquire() pattern - Use manager's defaultRetryAttempts/defaultRetryDelay as fallbacks - Report actual attempt count in error and handle metadata Fixes #62 * chore: bump version to 0.7.7
1 parent 0a1e1dc commit 85ce4ff

File tree

4 files changed

+293
-64
lines changed

4 files changed

+293
-64
lines changed

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "redlock-universal",
3-
"version": "0.7.5",
3+
"version": "0.7.7",
44
"description": "Production-ready distributed Redis locks for Node.js with support for both node-redis and ioredis clients",
55
"keywords": [
66
"redis",

src/manager/LockManager.ts

Lines changed: 117 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,10 @@ export class LockManager {
218218
* All locks are acquired atomically using a Lua script - either all succeed or none do.
219219
* Redis guarantees that Lua scripts execute atomically without interruption.
220220
*
221+
* **Retry Behavior:**
222+
* When a key is already locked, acquireBatch will retry according to the configured
223+
* retryAttempts and retryDelay options. This allows handling temporary lock contention.
224+
*
221225
* **Example:**
222226
* ```typescript
223227
* // Input: ['user:3', 'user:1', 'user:2']
@@ -229,15 +233,19 @@ export class LockManager {
229233
* @param options - Acquisition options
230234
* @param options.ttl - Lock time-to-live in milliseconds (defaults to manager's defaultTTL)
231235
* @param options.nodeIndex - Redis node index to use (defaults to 0)
236+
* @param options.retryAttempts - Number of retry attempts (defaults to manager's defaultRetryAttempts)
237+
* @param options.retryDelay - Delay between retries in milliseconds (defaults to manager's defaultRetryDelay)
232238
* @returns Promise resolving to array of lock handles in SORTED key order
233239
* @throws {Error} If keys array is empty or contains duplicates
234-
* @throws {LockAcquisitionError} If any key is already locked
240+
* @throws {LockAcquisitionError} If any key is already locked after all retry attempts
235241
*/
236242
async acquireBatch(
237243
keys: string[],
238244
options: {
239245
readonly ttl?: number;
240246
readonly nodeIndex?: number;
247+
readonly retryAttempts?: number;
248+
readonly retryDelay?: number;
241249
} = {}
242250
): Promise<LockHandle[]> {
243251
if (keys.length === 0) {
@@ -261,6 +269,8 @@ export class LockManager {
261269

262270
const adapter = this.config.nodes[nodeIndex]!;
263271
const ttl = options.ttl ?? this.config.defaultTTL;
272+
const retryAttempts = options.retryAttempts ?? this.config.defaultRetryAttempts;
273+
const retryDelay = options.retryDelay ?? this.config.defaultRetryDelay;
264274
const startTime = Date.now();
265275
const sortedKeys = [...keys].sort();
266276
const values = sortedKeys.map(() => generateLockValue());
@@ -273,81 +283,121 @@ export class LockManager {
273283
keys: sortedKeys.slice(0, 10), // Log first 10 to avoid huge logs
274284
ttl,
275285
nodeIndex,
286+
retryAttempts,
287+
retryDelay,
276288
});
277289
}
278290

279-
try {
280-
const result = await adapter.batchSetNX(sortedKeys, values, ttl);
281-
282-
if (!result.success) {
283-
this.stats.failedLocks += sortedKeys.length;
284-
285-
if (this.config.logger) {
286-
this.config.logger.error(
287-
'Batch lock acquisition failed (atomic guarantee preserved)',
288-
new Error('Lock acquisition failed'),
289-
{
290-
failedKey: result.failedKey,
291-
failedIndex: result.failedIndex,
292-
attemptedKeys: sortedKeys.length,
293-
acquisitionTime: Date.now() - startTime,
294-
}
295-
);
296-
}
291+
let lastError: Error | null = null;
292+
let lastFailedKey: string | undefined;
293+
let lastFailedIndex: number | undefined;
294+
295+
for (let attempt = 0; attempt <= retryAttempts; attempt++) {
296+
try {
297+
const result = await adapter.batchSetNX(sortedKeys, values, ttl);
298+
299+
if (result.success) {
300+
const acquisitionTime = Date.now() - startTime;
301+
302+
const handles: LockHandle[] = sortedKeys.map((key, index) => ({
303+
id: generateLockId(),
304+
key,
305+
value: values[index]!,
306+
acquiredAt: Date.now(),
307+
ttl,
308+
metadata: {
309+
attempts: attempt + 1,
310+
acquisitionTime,
311+
strategy: 'simple' as const,
312+
},
313+
}));
314+
315+
this.stats.acquisitionTimes.push(acquisitionTime);
316+
this.stats.acquiredLocks += handles.length;
317+
this.stats.activeLocks += handles.length;
318+
319+
for (const handle of handles) {
320+
this.activeLocks.set(handle.id, handle);
321+
}
297322

298-
throw new LockAcquisitionError(
299-
result.failedKey,
300-
1,
301-
new Error(
302-
`Batch acquisition failed: key "${result.failedKey}" at index ${result.failedIndex} is already locked`
303-
)
304-
);
305-
}
323+
if (this.config.logger) {
324+
this.config.logger.info('Batch lock acquisition succeeded', {
325+
lockCount: handles.length,
326+
acquisitionTime,
327+
avgTimePerLock: acquisitionTime / handles.length,
328+
attempts: attempt + 1,
329+
});
330+
}
306331

307-
const acquisitionTime = Date.now() - startTime;
332+
return handles;
333+
}
308334

309-
const handles: LockHandle[] = sortedKeys.map((key, index) => ({
310-
id: generateLockId(),
311-
key,
312-
value: values[index]!,
313-
acquiredAt: startTime,
314-
ttl,
315-
metadata: {
316-
attempts: 1,
317-
acquisitionTime,
318-
strategy: 'simple' as const,
319-
},
320-
}));
335+
// Acquisition failed - key is already locked
336+
lastFailedKey = result.failedKey;
337+
lastFailedIndex = result.failedIndex;
338+
lastError = new Error(
339+
`Batch acquisition failed: key "${result.failedKey}" at index ${result.failedIndex} is already locked`
340+
);
321341

322-
this.stats.acquisitionTimes.push(acquisitionTime);
323-
this.stats.acquiredLocks += handles.length;
324-
this.stats.activeLocks += handles.length;
342+
if (this.config.logger && attempt < retryAttempts) {
343+
this.config.logger.debug?.('Batch lock acquisition attempt failed, retrying', {
344+
attempt: attempt + 1,
345+
maxAttempts: retryAttempts + 1,
346+
failedKey: result.failedKey,
347+
failedIndex: result.failedIndex,
348+
retryDelay,
349+
});
350+
}
351+
} catch (error) {
352+
// Unexpected error (network, Redis, etc.)
353+
lastError = error as Error;
354+
lastFailedKey = sortedKeys[0];
355+
lastFailedIndex = 0;
325356

326-
for (const handle of handles) {
327-
this.activeLocks.set(handle.id, handle);
357+
if (this.config.logger && error instanceof Error) {
358+
this.config.logger.error('Unexpected error during batch acquisition attempt', error, {
359+
attempt: attempt + 1,
360+
maxAttempts: retryAttempts + 1,
361+
keyCount: sortedKeys.length,
362+
});
363+
}
328364
}
329365

330-
if (this.config.logger) {
331-
this.config.logger.info('Batch lock acquisition succeeded', {
332-
lockCount: handles.length,
333-
acquisitionTime,
334-
avgTimePerLock: acquisitionTime / handles.length,
335-
});
366+
// Wait before next retry (unless this was the last attempt)
367+
if (attempt < retryAttempts) {
368+
await this.sleep(retryDelay);
336369
}
370+
}
337371

338-
return handles;
339-
} catch (error) {
340-
if (!(error instanceof LockAcquisitionError)) {
341-
this.stats.failedLocks += sortedKeys.length;
372+
// All retries exhausted
373+
this.stats.failedLocks += sortedKeys.length;
342374

343-
if (this.config.logger && error instanceof Error) {
344-
this.config.logger.error('Unexpected error during batch acquisition', error, {
345-
keyCount: sortedKeys.length,
346-
});
375+
if (this.config.logger) {
376+
this.config.logger.error(
377+
'Batch lock acquisition failed after all retries',
378+
lastError ?? new Error('Lock acquisition failed'),
379+
{
380+
failedKey: lastFailedKey,
381+
failedIndex: lastFailedIndex,
382+
attemptedKeys: sortedKeys.length,
383+
totalAttempts: retryAttempts + 1,
384+
acquisitionTime: Date.now() - startTime,
347385
}
348-
}
349-
throw error;
386+
);
350387
}
388+
389+
throw new LockAcquisitionError(
390+
lastFailedKey ?? sortedKeys[0]!,
391+
retryAttempts + 1,
392+
lastError ?? new Error('Lock acquisition failed')
393+
);
394+
}
395+
396+
/**
397+
* Sleep for specified milliseconds
398+
*/
399+
private sleep(ms: number): Promise<void> {
400+
return new Promise(resolve => setTimeout(resolve, ms));
351401
}
352402

353403
/**
@@ -380,6 +430,10 @@ export class LockManager {
380430
* @param keys - Array of lock keys to acquire
381431
* @param routine - Function to execute while holding all locks
382432
* @param options - Lock configuration options
433+
* @param options.ttl - Lock time-to-live in milliseconds (defaults to manager's defaultTTL)
434+
* @param options.nodeIndex - Redis node index to use (defaults to 0)
435+
* @param options.retryAttempts - Number of retry attempts (defaults to manager's defaultRetryAttempts)
436+
* @param options.retryDelay - Delay between retries in milliseconds (defaults to manager's defaultRetryDelay)
383437
* @returns Promise resolving to the routine result
384438
*/
385439
async usingBatch<T>(
@@ -388,6 +442,8 @@ export class LockManager {
388442
options: {
389443
readonly ttl?: number;
390444
readonly nodeIndex?: number;
445+
readonly retryAttempts?: number;
446+
readonly retryDelay?: number;
391447
} = {}
392448
): Promise<T> {
393449
const handles = await this.acquireBatch(keys, options);

0 commit comments

Comments
 (0)