Skip to content

fix(iterator): only query when the cache is empty - #484

Open
dudanogueira wants to merge 3 commits into
weaviate:mainfrom
dudanogueira:fix/iterator-cache
Open

dudanogueira wants to merge 3 commits into
weaviate:mainfrom
dudanogueira:fix/iterator-cache

Conversation

@dudanogueira

@dudanogueira dudanogueira commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #483

collection.iterator() sent one Search request for every object it returned. Each call to next() fetched a page of 100 objects after the last UUID, kept only the first and threw the rest away. Results were still correct, but iterating over N objects took N round trips and moved about 100 × N objects over the wire, including vectors when includeVector is set.

Approach

The iterator already had a cache field; it was just being overwritten on every call. Now next() only queries when the cache is empty, then returns objects from it one at a time, moving the after cursor to the UUID of each object it returns. When a page is used up, the next query starts after the last object of that page. The loop still stops on the first empty page, so iterating over N objects takes ceil(N / 100) + 1 requests instead of N + 1.

This doesn't stop early on a page with fewer than 100 objects. That would save the final empty request, but only by assuming the server always fills a page when more data exists. Stopping on an empty page is the rule the iterator already relied on.

Risks

  • Page boundaries: the cursor is always the UUID of the last object actually returned, so no object is skipped or repeated. A test checks the exact (limit, after) sequence.
  • Staleness: objects are now served from a page fetched up to 99 next() calls earlier, so a change made mid-iteration may not show up until the next page. This is normal for cursor pagination.

Testing

New src/collections/iterator/unit.test.ts uses a mocked query function, so it needs no server:

  • For N = 0, 1, 99, 100, 101 and 250: all objects are returned in order, with exactly ceil(N / 100) + 1 query calls.
  • For 250 objects: the query calls are exactly (100, undefined), (100, 'uuid-99'), (100, 'uuid-199'), (100, 'uuid-249').

5 of the 7 cases fail on the previous implementation.

collection.iterator() sent one Search request per yielded object,
fetching 100 objects and discarding 99. Refill the cache only once it
is exhausted, so iterating N objects takes ceil(N / 100) + 1 requests.

Fixes weaviate#483

@orca-security-eu orca-security-eu Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Comment thread src/collections/iterator/index.ts Outdated
Comment on lines +18 to +23
if (this.cache.length == 0) {
return {
done: true,
value: undefined,
};
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, the double nesting is somewhat confusing here

can we have it so that we do?

if (this.cache.length == 0) { this.cache = await fetch }
if (this.cache.length == 0) { return done }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in df0b955, flattened to the two sequential checks as suggested. Thanks!

Move the iterator unit test to packages/test/node/iterator to follow the monorepo layout.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

collection.iterator() sends one Search request per object (fetches 100, uses 1)

2 participants