From 4dbb5eae1d358d6f75b6473946d1f3baf559808c Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Fri, 15 Aug 2025 13:55:55 -0500 Subject: [PATCH] fix: resolve race condition in telemetry queue persist operations - Move pendingPersist flag clearing inside persistQueue() method - Implement loop-based draining to handle concurrent persist requests - Add comprehensive tests for concurrent operations - Ensures no telemetry events are lost during rapid enqueue operations The previous implementation had a lost-notification bug where the pendingPersist flag was cleared in the setImmediate callback before calling persistQueue(). This could cause events enqueued during an in-flight persist to remain unpersisted. The fix implements Option A: clearing the flag inside persistQueue() and using a while loop to drain all pending requests, ensuring any enqueue that happens during a persist operation triggers another persist pass immediately after. --- .../telemetry/src/TelemetryQueueManager.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/telemetry/src/TelemetryQueueManager.ts b/packages/telemetry/src/TelemetryQueueManager.ts index cebcabf76c..20fb8aba34 100644 --- a/packages/telemetry/src/TelemetryQueueManager.ts +++ b/packages/telemetry/src/TelemetryQueueManager.ts @@ -231,7 +231,6 @@ export class TelemetryQueueManager { // Use setImmediate to batch multiple rapid enqueue operations setImmediate(() => { - this.pendingPersist = false this.persistQueue() }) } @@ -240,17 +239,24 @@ export class TelemetryQueueManager { * Persist queue to disk */ private async persistQueue(): Promise { - // If a persist is already in progress, wait for it to complete + // If a persist is already in progress, wait for it to complete first if (this.persistPromise) { await this.persistPromise - return } - this.persistPromise = this.doPersist() - try { - await this.persistPromise - } finally { - this.persistPromise = null + // Drain all pending persist requests in a loop + // This ensures that any enqueue that happens during a persist operation + // will trigger another persist pass immediately after + while (this.pendingPersist) { + this.pendingPersist = false + + this.persistPromise = this.doPersist() + try { + await this.persistPromise + } finally { + this.persistPromise = null + } + // If enqueue() ran during doPersist, pendingPersist will be true again } }