Skip to content

Commit

Permalink
fix: modify the BatchHttpLink to have each timer when the batch key i…
Browse files Browse the repository at this point in the history
…s different (#10408)

* fix: separate timer for each key
* Clean up unused scheduledBatchTimerByKey entries (Hat-tip to @kang-heewon)

Co-authored-by: Jeff Auriemma <bignimbus@users.noreply.github.com>
  • Loading branch information
zlrlo and bignimbus authored Jan 26, 2023
1 parent 47435e8 commit 55ffafc
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/silent-monkeys-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

fix: modify BatchHttpLink to have a separate timer for each different batch key
68 changes: 68 additions & 0 deletions src/link/batch/__tests__/batchLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,74 @@ describe('OperationBatcher', () => {
}
});

itAsync('should be able to consume from a queue containing multiple queries with different batch keys', (resolve, reject) => {
// NOTE: this test was added to ensure that queries don't "hang" when consumed by BatchLink.
// "Hanging" in this case results in this test never resolving. So
// if this test times out it's probably a real issue and not a flake
const request2: Operation = createOperation(
{},
{
query,
},
);

const BH = createMockBatchHandler(
{
request: { query },
result: { data },
},
{
request: { query },
result: { data },
},
);

let key = true;
const batchKey = () => {
key = !key;
return '' + !key;
};

const myBatcher = new OperationBatcher({
batchInterval: 10,
batchMax: 10,
batchHandler: BH,
batchKey
});

const observable1 = myBatcher.enqueueRequest({ operation });
const observable2 = myBatcher.enqueueRequest({ operation: request2 });

let notify = false;
observable1.subscribe(resultObj1 => {
try {
expect(resultObj1).toEqual({ data });
} catch (e) {
reject(e);
}

if (notify) {
resolve();
} else {
notify = true;
}
});

observable2.subscribe(resultObj2 => {
try {
expect(resultObj2).toEqual({ data });
} catch (e) {
reject(e);
}

if (notify) {
resolve();
} else {
notify = true;
}
});
});

itAsync('should return a promise when we enqueue a request and resolve it with a result', (resolve, reject) => {
const BH = createMockBatchHandler({
request: { query },
Expand Down
9 changes: 5 additions & 4 deletions src/link/batch/batching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class OperationBatcher {
// Queue on which the QueryBatcher will operate on a per-tick basis.
private batchesByKey = new Map<string, RequestBatch>();

private scheduledBatchTimer: ReturnType<typeof setTimeout>;
private scheduledBatchTimerByKey = new Map<string, ReturnType<typeof setTimeout>>();
private batchDebounce?: boolean;
private batchInterval?: number;
private batchMax: number;
Expand Down Expand Up @@ -211,9 +211,10 @@ export class OperationBatcher {
}

private scheduleQueueConsumption(key: string): void {
clearTimeout(this.scheduledBatchTimer);
this.scheduledBatchTimer = setTimeout(() => {
clearTimeout(this.scheduledBatchTimerByKey.get(key));
this.scheduledBatchTimerByKey.set(key, setTimeout(() => {
this.consumeQueue(key);
}, this.batchInterval);
this.scheduledBatchTimerByKey.delete(key);
}, this.batchInterval));
}
}

0 comments on commit 55ffafc

Please sign in to comment.