From d8f95da82efe74a17e9fd9849888ef5afa3c6237 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Jan 2026 16:46:40 +0000 Subject: [PATCH 01/10] Fix potential infinite loops in graph execution and data loading Add iteration safeguards to prevent infinite loops that can occur when using Electric with large datasets and ORDER BY/LIMIT queries: 1. `maybeRunGraph` while loop (collection-config-builder.ts): - Can loop infinitely when data loading triggers graph updates - Happens when WHERE filters out most data, causing dataNeeded() > 0 - Loading more data triggers updates that get filtered out - Added 10,000 iteration limit with error logging 2. `requestLimitedSnapshot` while loop (subscription.ts): - Can loop if index iteration has issues - Added 10,000 iteration limit with error logging - Removed unused `insertedKeys` tracking 3. `D2.run()` while loop (d2.ts): - Can loop infinitely on circular data flow bugs - Added 100,000 iteration limit with error logging The safeguards log errors to help debug the root cause while preventing the app from freezing. --- packages/db-ivm/src/d2.ts | 15 ++++++++++++++ packages/db/src/collection/subscription.ts | 18 +++++++++++++++-- .../query/live/collection-config-builder.ts | 20 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/db-ivm/src/d2.ts b/packages/db-ivm/src/d2.ts index 8451b2aff..7fe069ae5 100644 --- a/packages/db-ivm/src/d2.ts +++ b/packages/db-ivm/src/d2.ts @@ -57,7 +57,22 @@ export class D2 implements ID2 { } run(): void { + // Safety limit to prevent infinite loops in case of circular data flow + // or other bugs that cause operators to perpetually produce output. + // For legitimate pipelines, data should flow through in finite steps. + const MAX_RUN_ITERATIONS = 100000 + let iterations = 0 + while (this.pendingWork()) { + iterations++ + if (iterations > MAX_RUN_ITERATIONS) { + console.error( + `[D2 Graph] Execution exceeded ${MAX_RUN_ITERATIONS} iterations. ` + + `This may indicate an infinite loop in the dataflow graph. ` + + `Breaking out to prevent app freeze.`, + ) + break + } this.step() } } diff --git a/packages/db/src/collection/subscription.ts b/packages/db/src/collection/subscription.ts index 44c9af49f..ac88009af 100644 --- a/packages/db/src/collection/subscription.ts +++ b/packages/db/src/collection/subscription.ts @@ -502,8 +502,23 @@ export class CollectionSubscription ? compileExpression(new PropRef(orderByExpression.path), true) : null + // Safety limit to prevent infinite loops if the index iteration or filtering + // logic has issues. The loop should naturally terminate when the index is + // exhausted, but this provides a backstop. 10000 iterations is generous + // for any legitimate use case. + const MAX_SNAPSHOT_ITERATIONS = 10000 + let snapshotIterations = 0 + while (valuesNeeded() > 0 && !collectionExhausted()) { - const insertedKeys = new Set() // Track keys we add to `changes` in this iteration + snapshotIterations++ + if (snapshotIterations > MAX_SNAPSHOT_ITERATIONS) { + console.error( + `[TanStack DB] requestLimitedSnapshot exceeded ${MAX_SNAPSHOT_ITERATIONS} iterations. ` + + `This may indicate an infinite loop in index iteration or filtering. ` + + `Breaking out to prevent app freeze. Collection: ${this.collection.id}`, + ) + break + } for (const key of keys) { const value = this.collection.get(key)! @@ -515,7 +530,6 @@ export class CollectionSubscription // Extract the indexed value (e.g., salary) from the row, not the full row // This is needed for index.take() to work correctly with the BTree comparator biggestObservedValue = valueExtractor ? valueExtractor(value) : value - insertedKeys.add(key) // Track this key } keys = index.take(valuesNeeded(), biggestObservedValue, filterFn) diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index 21cd04d1d..50c533cf6 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -336,7 +336,27 @@ export class CollectionConfigBuilder< // Always run the graph if subscribed (eager execution) if (syncState.subscribedToAllCollections) { + // Safety limit to prevent infinite loops when data loading and graph processing + // create a feedback cycle. This can happen when: + // 1. OrderBy/limit queries filter out most data, causing dataNeeded() > 0 + // 2. Loading more data triggers updates that get filtered out + // 3. The cycle continues indefinitely + // 10000 iterations is generous for legitimate use cases but prevents hangs. + const MAX_GRAPH_ITERATIONS = 10000 + let iterations = 0 + while (syncState.graph.pendingWork()) { + iterations++ + if (iterations > MAX_GRAPH_ITERATIONS) { + console.error( + `[TanStack DB] Graph execution exceeded ${MAX_GRAPH_ITERATIONS} iterations. ` + + `This may indicate an infinite loop caused by data loading triggering ` + + `continuous graph updates. Breaking out of the loop to prevent app freeze. ` + + `Query ID: ${this.id}`, + ) + break + } + syncState.graph.run() // Flush accumulated changes after each graph step to commit them as one transaction. // This ensures intermediate join states (like null on one side) don't cause From 2a796ff87f1fb254c1057d97d54bd8626af95aa5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Jan 2026 16:57:17 +0000 Subject: [PATCH 02/10] Fix root cause of Electric infinite loop with ORDER BY/LIMIT queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The infinite loop occurred because `loadMoreIfNeeded` kept trying to load data even when the local index was exhausted. This happened when: 1. TopK had fewer items than limit (WHERE filtered out data) 2. loadMoreIfNeeded tried to load more → no local data found 3. Loop continued indefinitely since TopK still needed data Root cause fix: - Add `localIndexExhausted` flag to CollectionSubscriber - Track when local index has no more data for current cursor - Stop calling loadMoreIfNeeded when exhausted - Reset flag when new data arrives from sync layer (inserts) - requestLimitedSnapshot now returns boolean indicating if data was found Error handling improvements (per review feedback): - D2.run() now throws Error when iteration limit exceeded - Caller catches and calls transitionToError() for proper error state - requestLimitedSnapshot returns false when iteration limit hit - Live query properly shows error state if safeguard limits are hit This fixes the issue for both eager and progressive syncMode. --- packages/db-ivm/src/d2.ts | 11 ++--- packages/db/src/collection/subscription.ts | 17 +++++-- .../query/live/collection-config-builder.ts | 25 ++++++---- .../src/query/live/collection-subscriber.ts | 47 +++++++++++++++++-- 4 files changed, 76 insertions(+), 24 deletions(-) diff --git a/packages/db-ivm/src/d2.ts b/packages/db-ivm/src/d2.ts index 7fe069ae5..f8c5233d8 100644 --- a/packages/db-ivm/src/d2.ts +++ b/packages/db-ivm/src/d2.ts @@ -64,14 +64,11 @@ export class D2 implements ID2 { let iterations = 0 while (this.pendingWork()) { - iterations++ - if (iterations > MAX_RUN_ITERATIONS) { - console.error( - `[D2 Graph] Execution exceeded ${MAX_RUN_ITERATIONS} iterations. ` + - `This may indicate an infinite loop in the dataflow graph. ` + - `Breaking out to prevent app freeze.`, + if (++iterations > MAX_RUN_ITERATIONS) { + throw new Error( + `D2 graph execution exceeded ${MAX_RUN_ITERATIONS} iterations. ` + + `This may indicate an infinite loop in the dataflow graph.`, ) - break } this.step() } diff --git a/packages/db/src/collection/subscription.ts b/packages/db/src/collection/subscription.ts index ac88009af..785f9e846 100644 --- a/packages/db/src/collection/subscription.ts +++ b/packages/db/src/collection/subscription.ts @@ -410,13 +410,15 @@ export class CollectionSubscription * Note 1: it may load more rows than the provided LIMIT because it loads all values equal to the first cursor value + limit values greater. * This is needed to ensure that it does not accidentally skip duplicate values when the limit falls in the middle of some duplicated values. * Note 2: it does not send keys that have already been sent before. + * + * @returns true if local data was found and sent, false if the local index was exhausted */ requestLimitedSnapshot({ orderBy, limit, minValues, offset, - }: RequestLimitedSnapshotOptions) { + }: RequestLimitedSnapshotOptions): boolean { if (!limit) throw new Error(`limit is required`) if (!this.orderByIndex) { @@ -508,15 +510,16 @@ export class CollectionSubscription // for any legitimate use case. const MAX_SNAPSHOT_ITERATIONS = 10000 let snapshotIterations = 0 + let hitIterationLimit = false while (valuesNeeded() > 0 && !collectionExhausted()) { - snapshotIterations++ - if (snapshotIterations > MAX_SNAPSHOT_ITERATIONS) { + if (++snapshotIterations > MAX_SNAPSHOT_ITERATIONS) { console.error( `[TanStack DB] requestLimitedSnapshot exceeded ${MAX_SNAPSHOT_ITERATIONS} iterations. ` + `This may indicate an infinite loop in index iteration or filtering. ` + `Breaking out to prevent app freeze. Collection: ${this.collection.id}`, ) + hitIterationLimit = true break } @@ -611,6 +614,14 @@ export class CollectionSubscription // Track this loadSubset call this.loadedSubsets.push(loadOptions) this.trackLoadSubsetPromise(syncResult) + + // Return whether local data was found and iteration completed normally. + // Return false if: + // - No local data was found (index exhausted) + // - Iteration limit was hit (abnormal exit) + // Either case signals that the caller should stop trying to load more. + // The async loadSubset may still return data later. + return changes.length > 0 && !hitIterationLimit } // TODO: also add similar test but that checks that it can also load it from the collection's loadSubset function diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index 50c533cf6..9be18da77 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -346,18 +346,25 @@ export class CollectionConfigBuilder< let iterations = 0 while (syncState.graph.pendingWork()) { - iterations++ - if (iterations > MAX_GRAPH_ITERATIONS) { - console.error( - `[TanStack DB] Graph execution exceeded ${MAX_GRAPH_ITERATIONS} iterations. ` + - `This may indicate an infinite loop caused by data loading triggering ` + - `continuous graph updates. Breaking out of the loop to prevent app freeze. ` + - `Query ID: ${this.id}`, + if (++iterations > MAX_GRAPH_ITERATIONS) { + this.transitionToError( + `Graph execution exceeded ${MAX_GRAPH_ITERATIONS} iterations. ` + + `This likely indicates an infinite loop caused by data loading ` + + `triggering continuous graph updates.`, ) - break + return } - syncState.graph.run() + try { + syncState.graph.run() + } catch (error) { + // D2 graph throws when it exceeds its internal iteration limit + // Transition to error state so callers can detect incomplete data + this.transitionToError( + error instanceof Error ? error.message : String(error), + ) + return + } // Flush accumulated changes after each graph step to commit them as one transaction. // This ensures intermediate join states (like null on one side) don't cause // duplicate key errors when the full join result arrives in the same step. diff --git a/packages/db/src/query/live/collection-subscriber.ts b/packages/db/src/query/live/collection-subscriber.ts index ec4876b74..91af3858b 100644 --- a/packages/db/src/query/live/collection-subscriber.ts +++ b/packages/db/src/query/live/collection-subscriber.ts @@ -37,6 +37,12 @@ export class CollectionSubscriber< // can potentially send the same item to D2 multiple times. private sentToD2Keys = new Set() + // Track when the local index has been exhausted for the current cursor position. + // When true, loadMoreIfNeeded will not try to load more data until new data arrives. + // This prevents infinite loops when the TopK can't be filled because WHERE filters + // out all available data. + private localIndexExhausted = false + constructor( private alias: string, private collectionId: string, @@ -301,11 +307,25 @@ export class CollectionSubscriber< return true } + // If we've already exhausted the local index, don't try to load more. + // This prevents infinite loops when the TopK can't be filled because + // the WHERE clause filters out all available local data. + // The flag is reset when new data arrives from the sync layer. + if (this.localIndexExhausted) { + return true + } + // `dataNeeded` probes the orderBy operator to see if it needs more data // if it needs more data, it returns the number of items it needs const n = dataNeeded() if (n > 0) { - this.loadNextItems(n, subscription) + const foundLocalData = this.loadNextItems(n, subscription) + if (!foundLocalData) { + // No local data found - mark the index as exhausted so we don't + // keep trying in subsequent graph iterations. The sync layer's + // loadSubset has been called and may return data asynchronously. + this.localIndexExhausted = true + } } return true } @@ -320,7 +340,19 @@ export class CollectionSubscriber< return } - const trackedChanges = this.trackSentValues(changes, orderByInfo.comparator) + // Reset localIndexExhausted when new data arrives from the sync layer. + // This allows loadMoreIfNeeded to try loading again since there's new data. + // We only reset on inserts since updates/deletes don't add new data to load. + const changesArray = Array.isArray(changes) ? changes : [...changes] + const hasInserts = changesArray.some((c) => c.type === `insert`) + if (hasInserts) { + this.localIndexExhausted = false + } + + const trackedChanges = this.trackSentValues( + changesArray, + orderByInfo.comparator, + ) // Cache the loadMoreIfNeeded callback on the subscription using a symbol property. // This ensures we pass the same function instance to the scheduler each time, @@ -342,10 +374,14 @@ export class CollectionSubscriber< // Loads the next `n` items from the collection // starting from the biggest item it has sent - private loadNextItems(n: number, subscription: CollectionSubscription) { + // Returns true if local data was found, false if the local index is exhausted + private loadNextItems( + n: number, + subscription: CollectionSubscription, + ): boolean { const orderByInfo = this.getOrderByInfo() if (!orderByInfo) { - return + return false } const { orderBy, valueExtractorForRawRow, offset } = orderByInfo const biggestSentRow = this.biggest @@ -369,7 +405,8 @@ export class CollectionSubscriber< // Take the `n` items after the biggest sent value // Pass the current window offset to ensure proper deduplication - subscription.requestLimitedSnapshot({ + // Returns true if local data was found + return subscription.requestLimitedSnapshot({ orderBy: normalizedOrderBy, limit: n, minValues, From e698eb185ec5a043dd9a1a76577e1bd5155e0283 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Jan 2026 17:12:54 +0000 Subject: [PATCH 03/10] Add tests for infinite loop prevention with ORDER BY + LIMIT queries These tests verify the localIndexExhausted fix works correctly: 1. Does not infinite loop when WHERE filters out most data - Query wants 10 items, only 2 match - Verifies status !== 'error' (fix works, not just safeguard) 2. Resumes loading when new matching data arrives - Starts with 0 matching items - Insert new matching items - localIndexExhausted resets, loads new data 3. Handles updates that move items out of WHERE clause - Updates change values to no longer match WHERE - TopK correctly refills from remaining matching data --- .../db/tests/infinite-loop-prevention.test.ts | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 packages/db/tests/infinite-loop-prevention.test.ts diff --git a/packages/db/tests/infinite-loop-prevention.test.ts b/packages/db/tests/infinite-loop-prevention.test.ts new file mode 100644 index 000000000..403bd46de --- /dev/null +++ b/packages/db/tests/infinite-loop-prevention.test.ts @@ -0,0 +1,190 @@ +import { describe, expect, it } from 'vitest' +import { createCollection } from '../src/collection/index.js' +import { createLiveQueryCollection, gt } from '../src/query/index.js' +import { mockSyncCollectionOptions } from './utils.js' + +/** + * Tests for infinite loop prevention in ORDER BY + LIMIT queries. + * + * The issue: When a live query has ORDER BY + LIMIT, the TopK operator + * requests data until it has `limit` items. If the WHERE clause filters + * out most data, the TopK may never be filled, causing loadMoreIfNeeded + * to be called repeatedly in an infinite loop. + * + * The fix: CollectionSubscriber tracks when the local index is exhausted + * via `localIndexExhausted` flag, preventing repeated load attempts. + */ + +type TestItem = { + id: number + value: number + category: string +} + +describe(`Infinite loop prevention`, () => { + it(`should not infinite loop when WHERE filters out most data for ORDER BY + LIMIT query`, async () => { + // This test verifies that the localIndexExhausted optimization prevents + // unnecessary load attempts when the TopK can't be filled. + // + // The scenario: + // 1. Query wants 10 items with value > 90 + // 2. Only 2 items match (values 95 and 100) + // 3. Without the fix, loadMoreIfNeeded would keep trying to load more + // 4. With the fix, localIndexExhausted stops unnecessary attempts + + const initialData: Array = [] + for (let i = 1; i <= 20; i++) { + initialData.push({ + id: i, + value: i * 5, // values: 5, 10, 15, ... 95, 100 + category: i <= 10 ? `A` : `B`, + }) + } + + const sourceCollection = createCollection( + mockSyncCollectionOptions({ + id: `infinite-loop-test`, + getKey: (item: TestItem) => item.id, + initialData, + }), + ) + + await sourceCollection.preload() + + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 90)) + .orderBy(({ items }) => items.value, `desc`) + .limit(10) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + // Should complete without hanging or hitting safeguard + await liveQueryCollection.preload() + + // Verify results + const results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(2) + expect(results.map((r) => r.value)).toEqual([100, 95]) + + // Verify not in error state (didn't hit safeguard) + expect( + liveQueryCollection.status, + `Query should not be in error state`, + ).not.toBe(`error`) + }) + + it(`should resume loading when new matching data arrives`, async () => { + // Start with data that doesn't match WHERE clause + const initialData: Array = [ + { id: 1, value: 10, category: `A` }, + { id: 2, value: 20, category: `A` }, + { id: 3, value: 30, category: `A` }, + ] + + const { utils, ...options } = mockSyncCollectionOptions({ + id: `resume-loading-test`, + getKey: (item: TestItem) => item.id, + initialData, + }) + + const sourceCollection = createCollection(options) + await sourceCollection.preload() + + // Query wants items with value > 50, ordered by value, limit 5 + // Initially 0 items match + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 50)) + .orderBy(({ items }) => items.value, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + await liveQueryCollection.preload() + + // Should have 0 items initially + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(0) + + // Now add items that match the WHERE clause + utils.begin() + utils.write({ type: `insert`, value: { id: 4, value: 60, category: `B` } }) + utils.write({ type: `insert`, value: { id: 5, value: 70, category: `B` } }) + utils.commit() + + // Wait for changes to propagate + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Should now have 2 items (localIndexExhausted was reset by new inserts) + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(2) + expect(results.map((r) => r.value)).toEqual([70, 60]) + }) + + it(`should handle updates that move items out of WHERE clause`, async () => { + // All items initially match WHERE clause + const initialData: Array = [ + { id: 1, value: 100, category: `A` }, + { id: 2, value: 90, category: `A` }, + { id: 3, value: 80, category: `A` }, + { id: 4, value: 70, category: `A` }, + { id: 5, value: 60, category: `A` }, + ] + + const sourceCollection = createCollection( + mockSyncCollectionOptions({ + id: `update-out-of-where-test`, + getKey: (item: TestItem) => item.id, + initialData, + }), + ) + + await sourceCollection.preload() + + // Query: WHERE value > 50, ORDER BY value DESC, LIMIT 3 + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 50)) + .orderBy(({ items }) => items.value, `desc`) + .limit(3) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + await liveQueryCollection.preload() + + // Should have top 3: 100, 90, 80 + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(3) + expect(results.map((r) => r.value)).toEqual([100, 90, 80]) + + // Update items to move them OUT of WHERE clause + // This could trigger the infinite loop if not handled properly + sourceCollection.update(1, (draft) => { + draft.value = 40 // Now < 50, filtered out + }) + sourceCollection.update(2, (draft) => { + draft.value = 30 // Now < 50, filtered out + }) + + // Wait for changes to propagate + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Should now have: 80, 70, 60 (items 3, 4, 5) + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(3) + expect(results.map((r) => r.value)).toEqual([80, 70, 60]) + }) +}) From 9202d1c50e42e2e33be6ecff53b9d425fb0f61c2 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 22 Jan 2026 10:51:42 -0700 Subject: [PATCH 04/10] Add test that reproduces Electric infinite loop bug Adds a new test that directly reproduces the infinite loop bug by creating a collection with a custom loadSubset that synchronously injects updates matching the WHERE clause. This simulates Electric's behavior of sending continuous updates during graph execution. The test verifies: - Without the localIndexExhausted fix, loadSubset is called 100+ times (infinite loop) - With the fix, loadSubset is called < 10 times (loop terminates correctly) Also adds additional tests for edge cases around the localIndexExhausted flag. Co-Authored-By: Claude Opus 4.5 --- .../db/tests/infinite-loop-prevention.test.ts | 316 ++++++++++++++++++ 1 file changed, 316 insertions(+) diff --git a/packages/db/tests/infinite-loop-prevention.test.ts b/packages/db/tests/infinite-loop-prevention.test.ts index 403bd46de..a04bde961 100644 --- a/packages/db/tests/infinite-loop-prevention.test.ts +++ b/packages/db/tests/infinite-loop-prevention.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { createCollection } from '../src/collection/index.js' import { createLiveQueryCollection, gt } from '../src/query/index.js' import { mockSyncCollectionOptions } from './utils.js' +import type { SyncConfig } from '../src/index.js' /** * Tests for infinite loop prevention in ORDER BY + LIMIT queries. @@ -11,8 +12,17 @@ import { mockSyncCollectionOptions } from './utils.js' * out most data, the TopK may never be filled, causing loadMoreIfNeeded * to be called repeatedly in an infinite loop. * + * The infinite loop specifically occurs when: + * 1. Initial load exhausts the local index (TopK still needs more items) + * 2. Updates arrive (e.g., from Electric sync layer converting duplicate inserts to updates) + * 3. maybeRunGraph processes the update and calls loadMoreIfNeeded + * 4. loadMoreIfNeeded sees dataNeeded() > 0, calls loadNextItems + * 5. loadNextItems finds nothing (index exhausted), but without tracking this, + * the next iteration repeats steps 3-5 indefinitely + * * The fix: CollectionSubscriber tracks when the local index is exhausted * via `localIndexExhausted` flag, preventing repeated load attempts. + * The flag resets when new inserts arrive, allowing the system to try again. */ type TestItem = { @@ -22,6 +32,17 @@ type TestItem = { } describe(`Infinite loop prevention`, () => { + // The infinite loop bug occurs when: + // 1. Query has ORDER BY + LIMIT + WHERE that filters most data + // 2. Sync layer (like Electric) continuously sends updates + // 3. These updates trigger pendingWork() to remain true during maybeRunGraph + // 4. Without the localIndexExhausted fix, loadMoreIfNeeded keeps trying to load + // from the exhausted local index + // + // The last test ("should not infinite loop when loadSubset synchronously injects updates") + // directly reproduces the bug by creating a custom loadSubset that synchronously + // injects updates matching the WHERE clause. Without the fix, this causes an infinite loop. + it(`should not infinite loop when WHERE filters out most data for ORDER BY + LIMIT query`, async () => { // This test verifies that the localIndexExhausted optimization prevents // unnecessary load attempts when the TopK can't be filled. @@ -187,4 +208,299 @@ describe(`Infinite loop prevention`, () => { expect(results).toHaveLength(3) expect(results.map((r) => r.value)).toEqual([80, 70, 60]) }) + + it(`should not infinite loop when updates arrive after local index is exhausted`, async () => { + // This test simulates the Electric scenario where: + // 1. Initial data loads, but TopK can't be filled (WHERE filters too much) + // 2. Updates arrive from sync layer (like Electric converting duplicate inserts to updates) + // 3. Without the fix, each update would trigger loadMoreIfNeeded which tries + // to load from the exhausted local index, causing an infinite loop + // + // The fix: localIndexExhausted flag prevents repeated load attempts. + // The flag only resets when NEW INSERTS arrive (not updates/deletes). + + const initialData: Array = [] + for (let i = 1; i <= 10; i++) { + initialData.push({ + id: i, + value: i * 10, // values: 10, 20, 30, ... 100 + category: `A`, + }) + } + + const { utils, ...options } = mockSyncCollectionOptions({ + id: `electric-update-loop-test`, + getKey: (item: TestItem) => item.id, + initialData, + }) + + const sourceCollection = createCollection(options) + await sourceCollection.preload() + + // Query: WHERE value > 95, ORDER BY value DESC, LIMIT 5 + // Only item with value=100 matches, but we want 5 items + // This exhausts the local index after the first item + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 95)) + .orderBy(({ items }) => items.value, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + // Preload should complete without hanging + const preloadPromise = liveQueryCollection.preload() + const timeoutPromise = new Promise((_, reject) => + setTimeout( + () => reject(new Error(`Timeout during preload - possible infinite loop`)), + 5000, + ), + ) + + await expect( + Promise.race([preloadPromise, timeoutPromise]), + ).resolves.toBeUndefined() + + // Should have 1 item (value=100) + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + expect(results[0]!.value).toBe(100) + + // Now simulate Electric sending updates (like duplicate insert → update conversion) + // Without the fix, this would trigger infinite loop because: + // 1. Update arrives, triggers maybeRunGraph + // 2. loadMoreIfNeeded sees dataNeeded() > 0 (TopK still needs 4 more) + // 3. loadNextItems finds nothing (index exhausted) + // 4. Without localIndexExhausted flag, loop would repeat indefinitely + const updatePromise = (async () => { + // Send several updates that don't change the result set + // These simulate Electric's duplicate handling + for (let i = 0; i < 5; i++) { + utils.begin() + // Update an item that doesn't match WHERE - this shouldn't affect results + // but could trigger the infinite loop bug + utils.write({ + type: `update`, + value: { id: 5, value: 50 + i, category: `A` }, // Still doesn't match WHERE + }) + utils.commit() + + // Small delay between updates to simulate real Electric behavior + await new Promise((resolve) => setTimeout(resolve, 10)) + } + })() + + const updateTimeoutPromise = new Promise((_, reject) => + setTimeout( + () => reject(new Error(`Timeout during updates - possible infinite loop`)), + 5000, + ), + ) + + await expect( + Promise.race([updatePromise, updateTimeoutPromise]), + ).resolves.toBeUndefined() + + // Results should still be the same (updates didn't add matching items) + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + expect(results[0]!.value).toBe(100) + }) + + it(`should reset localIndexExhausted when new inserts arrive`, async () => { + // This test verifies that the localIndexExhausted flag properly resets + // when new inserts arrive, allowing the system to load more data + + const { utils, ...options } = mockSyncCollectionOptions({ + id: `reset-exhausted-flag-test`, + getKey: (item: TestItem) => item.id, + initialData: [ + { id: 1, value: 100, category: `A` }, + ], + }) + + const sourceCollection = createCollection(options) + await sourceCollection.preload() + + // Query: WHERE value > 50, ORDER BY value DESC, LIMIT 5 + // Initially only 1 item matches, but we want 5 + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 50)) + .orderBy(({ items }) => items.value, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + await liveQueryCollection.preload() + + // Should have 1 item initially + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + + // Send updates (should NOT reset the flag, should NOT trigger more loads) + utils.begin() + utils.write({ type: `update`, value: { id: 1, value: 101, category: `A` } }) + utils.commit() + + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Still 1 item (updated value) + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + expect(results[0]!.value).toBe(101) + + // Now send NEW INSERTS - this SHOULD reset the flag and load more + utils.begin() + utils.write({ type: `insert`, value: { id: 2, value: 90, category: `B` } }) + utils.write({ type: `insert`, value: { id: 3, value: 80, category: `B` } }) + utils.commit() + + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Now should have 3 items (new inserts reset the flag, allowing more to load) + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(3) + expect(results.map((r) => r.value)).toEqual([101, 90, 80]) + }) + + it(`should not infinite loop when loadSubset synchronously injects updates (simulates Electric)`, async () => { + // This test reproduces the actual infinite loop bug by simulating Electric's behavior: + // - Electric's sync layer can call loadSubset which synchronously injects updates + // - These updates add data to D2, making pendingWork() return true + // - Without localIndexExhausted, loadMoreIfNeeded keeps trying to load from exhausted index + // - The synchronous update injection keeps pendingWork() true → infinite loop + // + // The fix: localIndexExhausted flag prevents repeated load attempts after index is exhausted + + const initialData: Array = [ + { id: 1, value: 100, category: `A` }, // Only this matches WHERE > 95 + { id: 2, value: 50, category: `A` }, + { id: 3, value: 40, category: `A` }, + ] + + // Track how many times loadSubset is called to detect infinite loop + let loadSubsetCallCount = 0 + const MAX_LOADSUBSET_CALLS = 100 // Safety limit + + // Store sync params for injecting updates in loadSubset + let syncBegin: () => void + let syncWrite: (msg: { type: string; value: TestItem }) => void + let syncCommit: () => void + let updateCounter = 0 + + const sync: SyncConfig = { + sync: (params) => { + syncBegin = params.begin + syncWrite = params.write as typeof syncWrite + syncCommit = params.commit + const markReady = params.markReady + + // Load initial data + syncBegin() + initialData.forEach((item) => { + syncWrite({ type: `insert`, value: item }) + }) + syncCommit() + markReady() + + return { + // This loadSubset function simulates Electric's behavior: + // When the subscription asks for more data (because TopK isn't full), + // Electric might synchronously send an update for existing data + loadSubset: () => { + loadSubsetCallCount++ + + if (loadSubsetCallCount > MAX_LOADSUBSET_CALLS) { + throw new Error( + `loadSubset called ${loadSubsetCallCount} times - infinite loop detected!`, + ) + } + + // Simulate Electric sending an update for a matching item + // This is key: the update must match WHERE clause to pass the subscription filter + // and actually add work to D2, keeping pendingWork() true + syncBegin() + syncWrite({ + type: `update`, + value: { id: 1, value: 100 + updateCounter++, category: `A` }, // Matches WHERE > 95 + }) + syncCommit() + + return true // Synchronous completion + }, + } + }, + } + + const sourceCollection = createCollection({ + id: `loadsubset-infinite-loop-test`, + getKey: (item: TestItem) => item.id, + sync, + syncMode: `on-demand`, + }) + + await sourceCollection.preload() + + // Query: WHERE value > 95, ORDER BY value DESC, LIMIT 5 + // Only 1 item matches (value=100), but we want 5 + // This will exhaust the local index and trigger loadSubset calls + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 95)) + .orderBy(({ items }) => items.value, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + // Without the fix, this would infinite loop because: + // 1. Initial load finds 1 item, TopK needs 4 more + // 2. loadSubset is called, synchronously injects an update + // 3. Update adds D2 work, pendingWork() returns true + // 4. maybeRunGraph continues, loadMoreIfNeeded is called + // 5. TopK still needs 4 more, loadNextItems finds nothing (index exhausted) + // 6. But wait! We're back to step 2 because pendingWork() is still true from the update + // 7. GOTO step 2 → infinite loop! + // + // With the fix: localIndexExhausted flag is set in step 5, preventing step 6 + + const preloadPromise = liveQueryCollection.preload() + const timeoutPromise = new Promise((_, reject) => + setTimeout( + () => + reject( + new Error( + `Timeout - possible infinite loop. loadSubset was called ${loadSubsetCallCount} times.`, + ), + ), + 5000, + ), + ) + + await expect( + Promise.race([preloadPromise, timeoutPromise]), + ).resolves.toBeUndefined() + + // Verify we got the expected result + const results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + // Value will be 100 + number of loadSubset calls (minus 1 since counter starts at 0) + expect(results[0]!.value).toBeGreaterThanOrEqual(100) + + // loadSubset should be called a small number of times, not infinitely + // The exact count depends on implementation details, but should be < 10 + expect(loadSubsetCallCount).toBeLessThan(10) + }) }) From d43a509241d96c15a64b72bbb54ac775bc12b126 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 22 Jan 2026 17:52:54 +0000 Subject: [PATCH 05/10] ci: apply automated fixes --- packages/db/tests/infinite-loop-prevention.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/db/tests/infinite-loop-prevention.test.ts b/packages/db/tests/infinite-loop-prevention.test.ts index a04bde961..969c6c2ad 100644 --- a/packages/db/tests/infinite-loop-prevention.test.ts +++ b/packages/db/tests/infinite-loop-prevention.test.ts @@ -256,7 +256,8 @@ describe(`Infinite loop prevention`, () => { const preloadPromise = liveQueryCollection.preload() const timeoutPromise = new Promise((_, reject) => setTimeout( - () => reject(new Error(`Timeout during preload - possible infinite loop`)), + () => + reject(new Error(`Timeout during preload - possible infinite loop`)), 5000, ), ) @@ -296,7 +297,8 @@ describe(`Infinite loop prevention`, () => { const updateTimeoutPromise = new Promise((_, reject) => setTimeout( - () => reject(new Error(`Timeout during updates - possible infinite loop`)), + () => + reject(new Error(`Timeout during updates - possible infinite loop`)), 5000, ), ) @@ -318,9 +320,7 @@ describe(`Infinite loop prevention`, () => { const { utils, ...options } = mockSyncCollectionOptions({ id: `reset-exhausted-flag-test`, getKey: (item: TestItem) => item.id, - initialData: [ - { id: 1, value: 100, category: `A` }, - ], + initialData: [{ id: 1, value: 100, category: `A` }], }) const sourceCollection = createCollection(options) From e6a9fbc56ceb539004949c207e46d3cc8ef3454e Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 22 Jan 2026 11:01:07 -0700 Subject: [PATCH 06/10] Add changeset for infinite loop fix Co-Authored-By: Claude Opus 4.5 --- .changeset/fix-infinite-loop-orderby-limit.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/fix-infinite-loop-orderby-limit.md diff --git a/.changeset/fix-infinite-loop-orderby-limit.md b/.changeset/fix-infinite-loop-orderby-limit.md new file mode 100644 index 000000000..5d0740499 --- /dev/null +++ b/.changeset/fix-infinite-loop-orderby-limit.md @@ -0,0 +1,6 @@ +--- +'@tanstack/db': patch +'@tanstack/db-ivm': patch +--- + +Fix infinite loop in ORDER BY + LIMIT queries when WHERE clause filters out most data. Add `localIndexExhausted` flag to prevent repeated load attempts when the local index is exhausted. Also add safety iteration limits to D2 graph execution, maybeRunGraph, and requestLimitedSnapshot as backstops. From dc3d40c63268d77df0f0424fccab49f413f73162 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 22 Jan 2026 11:30:27 -0700 Subject: [PATCH 07/10] Remove flawed test that doesn't reproduce real Electric bug The removed test synchronously injected data in loadSubset, which artificially creates an infinite loop. Electric's loadSubset is async (uses await stream.fetchSnapshot), so it can't synchronously inject data during the maybeRunGraph loop. The remaining tests verify the localIndexExhausted flag's behavior correctly: - Prevents repeated load attempts when exhausted - Resets when new inserts arrive Co-Authored-By: Claude Opus 4.5 --- .../db/tests/infinite-loop-prevention.test.ts | 147 ++---------------- 1 file changed, 12 insertions(+), 135 deletions(-) diff --git a/packages/db/tests/infinite-loop-prevention.test.ts b/packages/db/tests/infinite-loop-prevention.test.ts index 969c6c2ad..c8d8b0f7f 100644 --- a/packages/db/tests/infinite-loop-prevention.test.ts +++ b/packages/db/tests/infinite-loop-prevention.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from 'vitest' import { createCollection } from '../src/collection/index.js' import { createLiveQueryCollection, gt } from '../src/query/index.js' import { mockSyncCollectionOptions } from './utils.js' -import type { SyncConfig } from '../src/index.js' /** * Tests for infinite loop prevention in ORDER BY + LIMIT queries. @@ -39,9 +38,9 @@ describe(`Infinite loop prevention`, () => { // 4. Without the localIndexExhausted fix, loadMoreIfNeeded keeps trying to load // from the exhausted local index // - // The last test ("should not infinite loop when loadSubset synchronously injects updates") - // directly reproduces the bug by creating a custom loadSubset that synchronously - // injects updates matching the WHERE clause. Without the fix, this causes an infinite loop. + // These tests verify the localIndexExhausted flag works correctly: + // - Prevents repeated load attempts when the local index is exhausted + // - Resets when new inserts arrive, allowing the system to try again it(`should not infinite loop when WHERE filters out most data for ORDER BY + LIMIT query`, async () => { // This test verifies that the localIndexExhausted optimization prevents @@ -372,135 +371,13 @@ describe(`Infinite loop prevention`, () => { expect(results.map((r) => r.value)).toEqual([101, 90, 80]) }) - it(`should not infinite loop when loadSubset synchronously injects updates (simulates Electric)`, async () => { - // This test reproduces the actual infinite loop bug by simulating Electric's behavior: - // - Electric's sync layer can call loadSubset which synchronously injects updates - // - These updates add data to D2, making pendingWork() return true - // - Without localIndexExhausted, loadMoreIfNeeded keeps trying to load from exhausted index - // - The synchronous update injection keeps pendingWork() true → infinite loop - // - // The fix: localIndexExhausted flag prevents repeated load attempts after index is exhausted - - const initialData: Array = [ - { id: 1, value: 100, category: `A` }, // Only this matches WHERE > 95 - { id: 2, value: 50, category: `A` }, - { id: 3, value: 40, category: `A` }, - ] - - // Track how many times loadSubset is called to detect infinite loop - let loadSubsetCallCount = 0 - const MAX_LOADSUBSET_CALLS = 100 // Safety limit - - // Store sync params for injecting updates in loadSubset - let syncBegin: () => void - let syncWrite: (msg: { type: string; value: TestItem }) => void - let syncCommit: () => void - let updateCounter = 0 - - const sync: SyncConfig = { - sync: (params) => { - syncBegin = params.begin - syncWrite = params.write as typeof syncWrite - syncCommit = params.commit - const markReady = params.markReady - - // Load initial data - syncBegin() - initialData.forEach((item) => { - syncWrite({ type: `insert`, value: item }) - }) - syncCommit() - markReady() - - return { - // This loadSubset function simulates Electric's behavior: - // When the subscription asks for more data (because TopK isn't full), - // Electric might synchronously send an update for existing data - loadSubset: () => { - loadSubsetCallCount++ - - if (loadSubsetCallCount > MAX_LOADSUBSET_CALLS) { - throw new Error( - `loadSubset called ${loadSubsetCallCount} times - infinite loop detected!`, - ) - } - - // Simulate Electric sending an update for a matching item - // This is key: the update must match WHERE clause to pass the subscription filter - // and actually add work to D2, keeping pendingWork() true - syncBegin() - syncWrite({ - type: `update`, - value: { id: 1, value: 100 + updateCounter++, category: `A` }, // Matches WHERE > 95 - }) - syncCommit() - - return true // Synchronous completion - }, - } - }, - } - - const sourceCollection = createCollection({ - id: `loadsubset-infinite-loop-test`, - getKey: (item: TestItem) => item.id, - sync, - syncMode: `on-demand`, - }) - - await sourceCollection.preload() - - // Query: WHERE value > 95, ORDER BY value DESC, LIMIT 5 - // Only 1 item matches (value=100), but we want 5 - // This will exhaust the local index and trigger loadSubset calls - const liveQueryCollection = createLiveQueryCollection((q) => - q - .from({ items: sourceCollection }) - .where(({ items }) => gt(items.value, 95)) - .orderBy(({ items }) => items.value, `desc`) - .limit(5) - .select(({ items }) => ({ - id: items.id, - value: items.value, - })), - ) - - // Without the fix, this would infinite loop because: - // 1. Initial load finds 1 item, TopK needs 4 more - // 2. loadSubset is called, synchronously injects an update - // 3. Update adds D2 work, pendingWork() returns true - // 4. maybeRunGraph continues, loadMoreIfNeeded is called - // 5. TopK still needs 4 more, loadNextItems finds nothing (index exhausted) - // 6. But wait! We're back to step 2 because pendingWork() is still true from the update - // 7. GOTO step 2 → infinite loop! - // - // With the fix: localIndexExhausted flag is set in step 5, preventing step 6 - - const preloadPromise = liveQueryCollection.preload() - const timeoutPromise = new Promise((_, reject) => - setTimeout( - () => - reject( - new Error( - `Timeout - possible infinite loop. loadSubset was called ${loadSubsetCallCount} times.`, - ), - ), - 5000, - ), - ) - - await expect( - Promise.race([preloadPromise, timeoutPromise]), - ).resolves.toBeUndefined() - - // Verify we got the expected result - const results = Array.from(liveQueryCollection.values()) - expect(results).toHaveLength(1) - // Value will be 100 + number of loadSubset calls (minus 1 since counter starts at 0) - expect(results[0]!.value).toBeGreaterThanOrEqual(100) - - // loadSubset should be called a small number of times, not infinitely - // The exact count depends on implementation details, but should be < 10 - expect(loadSubsetCallCount).toBeLessThan(10) - }) + // NOTE: The actual Electric infinite loop is difficult to reproduce in unit tests because + // Electric's loadSubset is async (uses `await stream.fetchSnapshot`), so it can't + // synchronously inject data during the maybeRunGraph loop. The exact conditions that + // cause the infinite loop in production involve timing-dependent interactions between + // async data arrival and graph execution that are hard to simulate deterministically. + // + // The localIndexExhausted fix prevents unnecessary repeated load attempts regardless + // of whether the trigger is sync or async. The tests above verify the flag's behavior + // correctly: it prevents repeated loads when exhausted, and resets when new inserts arrive. }) From cb07f2b732dc90696c00e3d479c288dcf6699915 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 22 Jan 2026 11:34:23 -0700 Subject: [PATCH 08/10] Fix localIndexExhausted resetting on updates and add verification test The localIndexExhausted flag was incorrectly resetting when updates arrived because splitUpdates converts updates to delete+insert pairs for D2, and the hasInserts check was seeing those fake inserts. Fix: Check for ORIGINAL inserts before calling splitUpdates, and pass that information to sendChangesToPipelineWithTracking. Now the flag only resets for genuine new inserts from the sync layer. Also adds a test that verifies requestLimitedSnapshot calls are limited after the index is exhausted - with the fix, only 0-4 calls happen after initial load even when 20 updates arrive. Without the fix, it would be ~19 calls. Co-Authored-By: Claude Opus 4.5 --- .../src/query/live/collection-subscriber.ts | 20 +++- .../db/tests/infinite-loop-prevention.test.ts | 109 ++++++++++++++++-- 2 files changed, 115 insertions(+), 14 deletions(-) diff --git a/packages/db/src/query/live/collection-subscriber.ts b/packages/db/src/query/live/collection-subscriber.ts index 91af3858b..d08b837b4 100644 --- a/packages/db/src/query/live/collection-subscriber.ts +++ b/packages/db/src/query/live/collection-subscriber.ts @@ -228,11 +228,19 @@ export class CollectionSubscriber< const sendChangesInRange = ( changes: Iterable>, ) => { + // Check for ORIGINAL inserts before splitting updates. + // splitUpdates converts updates to delete+insert pairs for D2, but those + // fake inserts shouldn't reset localIndexExhausted. Only genuine new inserts + // (new data from the sync layer) should reset it. + const changesArray = Array.isArray(changes) ? changes : [...changes] + const hasOriginalInserts = changesArray.some((c) => c.type === `insert`) + // Split live updates into a delete of the old value and an insert of the new value - const splittedChanges = splitUpdates(changes) + const splittedChanges = splitUpdates(changesArray) this.sendChangesToPipelineWithTracking( splittedChanges, subscriptionHolder.current!, + hasOriginalInserts, ) } @@ -333,6 +341,7 @@ export class CollectionSubscriber< private sendChangesToPipelineWithTracking( changes: Iterable>, subscription: CollectionSubscription, + hasOriginalInserts?: boolean, ) { const orderByInfo = this.getOrderByInfo() if (!orderByInfo) { @@ -340,12 +349,13 @@ export class CollectionSubscriber< return } - // Reset localIndexExhausted when new data arrives from the sync layer. + // Reset localIndexExhausted when genuinely new data arrives from the sync layer. // This allows loadMoreIfNeeded to try loading again since there's new data. - // We only reset on inserts since updates/deletes don't add new data to load. + // We only reset on ORIGINAL inserts - not fake inserts from splitUpdates. + // splitUpdates converts updates to delete+insert for D2, but those shouldn't + // reset the flag since they don't represent new data that could fill the TopK. const changesArray = Array.isArray(changes) ? changes : [...changes] - const hasInserts = changesArray.some((c) => c.type === `insert`) - if (hasInserts) { + if (hasOriginalInserts) { this.localIndexExhausted = false } diff --git a/packages/db/tests/infinite-loop-prevention.test.ts b/packages/db/tests/infinite-loop-prevention.test.ts index c8d8b0f7f..f1a9ef0eb 100644 --- a/packages/db/tests/infinite-loop-prevention.test.ts +++ b/packages/db/tests/infinite-loop-prevention.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { createCollection } from '../src/collection/index.js' import { createLiveQueryCollection, gt } from '../src/query/index.js' +import { CollectionSubscription } from '../src/collection/subscription.js' import { mockSyncCollectionOptions } from './utils.js' /** @@ -371,13 +372,103 @@ describe(`Infinite loop prevention`, () => { expect(results.map((r) => r.value)).toEqual([101, 90, 80]) }) - // NOTE: The actual Electric infinite loop is difficult to reproduce in unit tests because - // Electric's loadSubset is async (uses `await stream.fetchSnapshot`), so it can't - // synchronously inject data during the maybeRunGraph loop. The exact conditions that - // cause the infinite loop in production involve timing-dependent interactions between - // async data arrival and graph execution that are hard to simulate deterministically. - // - // The localIndexExhausted fix prevents unnecessary repeated load attempts regardless - // of whether the trigger is sync or async. The tests above verify the flag's behavior - // correctly: it prevents repeated loads when exhausted, and resets when new inserts arrive. + it(`should limit requestLimitedSnapshot calls when index is exhausted`, async () => { + // This test verifies that the localIndexExhausted optimization actually limits + // how many times we try to load from an exhausted index. + // + // We patch CollectionSubscription.prototype.requestLimitedSnapshot to count calls, + // then send multiple updates and verify the call count stays low (not unbounded). + + // Patch prototype before creating anything + let requestLimitedSnapshotCallCount = 0 + const originalRequestLimitedSnapshot = + CollectionSubscription.prototype.requestLimitedSnapshot + + CollectionSubscription.prototype.requestLimitedSnapshot = function ( + ...args: Array + ) { + requestLimitedSnapshotCallCount++ + return originalRequestLimitedSnapshot.apply(this, args as any) + } + + try { + const initialData: Array = [ + { id: 1, value: 100, category: `A` }, // Only this matches WHERE > 95 + { id: 2, value: 50, category: `A` }, + { id: 3, value: 40, category: `A` }, + ] + + const { utils, ...options } = mockSyncCollectionOptions({ + id: `limited-snapshot-calls-test`, + getKey: (item: TestItem) => item.id, + initialData, + }) + + const sourceCollection = createCollection(options) + await sourceCollection.preload() + + // Query: WHERE value > 95, ORDER BY value DESC, LIMIT 5 + // Only 1 item matches (value=100), but we want 5 + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 95)) + .orderBy(({ items }) => items.value, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + await liveQueryCollection.preload() + + // Record how many calls happened during initial load + const initialLoadCalls = requestLimitedSnapshotCallCount + + // Should have 1 item initially + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + expect(results[0]!.value).toBe(100) + + // Send 20 updates that match the WHERE clause + // Without the fix, each update would trigger loadMoreIfNeeded which would + // call requestLimitedSnapshot. With the fix, localIndexExhausted prevents + // repeated calls. + for (let i = 0; i < 20; i++) { + utils.begin() + utils.write({ + type: `update`, + value: { id: 1, value: 100 + i, category: `A` }, + }) + utils.commit() + await new Promise((resolve) => setTimeout(resolve, 5)) + } + + // Wait for all processing to complete + await new Promise((resolve) => setTimeout(resolve, 100)) + + // Calculate calls after the updates + const callsAfterUpdates = + requestLimitedSnapshotCallCount - initialLoadCalls + + // With the fix, requestLimitedSnapshot should be called very few times + // after the initial load (ideally 0 since index was already exhausted) + // Without the fix, it would be called ~20 times (once per update) + expect(callsAfterUpdates).toBeLessThan(5) + + // Results should show the latest value + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + expect(results[0]!.value).toBeGreaterThanOrEqual(100) + } finally { + // Restore original method + CollectionSubscription.prototype.requestLimitedSnapshot = + originalRequestLimitedSnapshot + } + }) + + // NOTE: The actual Electric infinite loop involves async timing that's hard to reproduce + // in unit tests. The test above verifies the optimization limits repeated calls, + // which is the core behavior the localIndexExhausted flag provides. }) From 6e35ac5f44151500bacd678ec41fc4c37b2b6807 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 22 Jan 2026 11:38:53 -0700 Subject: [PATCH 09/10] Add detailed comment explaining why reset-only-on-inserts is correct Documents the reasoning for external reviewers: 1. splitUpdates fake inserts don't reset the flag 2. Updates to existing rows don't add new rows to scan 3. Edge case "update makes row match WHERE" is handled by filterAndFlipChanges converting unseen key updates to inserts Co-Authored-By: Claude Opus 4.5 --- .../src/query/live/collection-subscriber.ts | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/db/src/query/live/collection-subscriber.ts b/packages/db/src/query/live/collection-subscriber.ts index d08b837b4..428a562a2 100644 --- a/packages/db/src/query/live/collection-subscriber.ts +++ b/packages/db/src/query/live/collection-subscriber.ts @@ -228,10 +228,22 @@ export class CollectionSubscriber< const sendChangesInRange = ( changes: Iterable>, ) => { - // Check for ORIGINAL inserts before splitting updates. - // splitUpdates converts updates to delete+insert pairs for D2, but those - // fake inserts shouldn't reset localIndexExhausted. Only genuine new inserts - // (new data from the sync layer) should reset it. + // Check for inserts before splitting updates, to determine if we should + // reset localIndexExhausted. We reset on inserts because: + // + // 1. splitUpdates (below) converts updates to delete+insert pairs for D2, + // but those "fake" inserts shouldn't reset the flag - they don't represent + // new rows that could fill the TopK. + // + // 2. "Reset only on inserts" is correct because updates to existing rows + // don't add new rows to scan in the local index. The updated row is + // already being processed in the current graph run. + // + // 3. Edge case: "update makes row match WHERE" is handled correctly because + // the subscription's filterAndFlipChanges converts "update for unseen key" + // to "insert" before we receive it here. So if a row that was previously + // filtered out by WHERE now matches after an update, it arrives as an + // insert and correctly resets the flag. const changesArray = Array.isArray(changes) ? changes : [...changes] const hasOriginalInserts = changesArray.some((c) => c.type === `insert`) From 3a5f05d4238b11110ebfbf5d2bbae416135e471f Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 22 Jan 2026 12:10:26 -0700 Subject: [PATCH 10/10] Update changeset with clearer ELI5 explanation Co-Authored-By: Claude Opus 4.5 --- .changeset/fix-infinite-loop-orderby-limit.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.changeset/fix-infinite-loop-orderby-limit.md b/.changeset/fix-infinite-loop-orderby-limit.md index 5d0740499..37d80a22d 100644 --- a/.changeset/fix-infinite-loop-orderby-limit.md +++ b/.changeset/fix-infinite-loop-orderby-limit.md @@ -3,4 +3,10 @@ '@tanstack/db-ivm': patch --- -Fix infinite loop in ORDER BY + LIMIT queries when WHERE clause filters out most data. Add `localIndexExhausted` flag to prevent repeated load attempts when the local index is exhausted. Also add safety iteration limits to D2 graph execution, maybeRunGraph, and requestLimitedSnapshot as backstops. +Fix infinite loop in ORDER BY + LIMIT queries when WHERE filters out most data. + +**The problem**: Query asks for "top 10 where category='rare'" but only 3 rare items exist locally. System keeps asking "give me more!" but local index has nothing else. Loop forever. + +**The fix**: Added `localIndexExhausted` flag. When local index says "nothing left," we remember and stop asking. Flag resets when genuinely new data arrives from sync layer. + +Also adds safety iteration limits as backstops (D2: 100k, maybeRunGraph: 10k, requestLimitedSnapshot: 10k).