Repository navigation
fix(bigtable): resolve O(n^2) bottleneck in ReadRowsQuery.add_range - #18706
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the performance of ReadRowsQuery.add_range from O(N) to O(1) by making RowRange hashable and maintaining a _range_signatures set. It also fixes a typo from slots to __slots__ to ensure proper memory optimization, and adds corresponding unit tests. However, the proposed self-healing logic in add_range compares the lengths of the _range_signatures set and the _row_set.row_ranges list. If the query is initialized with duplicate ranges, these lengths will never match, causing the cache to rebuild on every call and introducing an O(N^2) bottleneck. It is recommended to remove this self-healing check entirely.
c486b34 to
f6a2fa6
Compare
f6a2fa6 to
f8d3b11
Compare
ohmayr
left a comment
ohmayr
left a comment
There was a problem hiding this comment.
Thanks for picking this up. Using a set is the right fix. One blocking issue (inline): __hash__ and __eq__ disagree for the legacy row_set.RowRange, so mixed legacy/base ranges no longer dedup. The rest are small.
Separately, add_key and ReadRowsQuery.__eq__ have the same O(n²) pattern: 10k add_key calls take ~11.6s, and __eq__ on two 2k-range queries takes ~50s. These can go in a follow-up PR, but please don't close b/573522284 until they're tracked, since key-heavy workloads are still slow.
…and deduplicate in __init__
I've addressed both points directly in this PR (we can fully resolve Fixed in ef09fca (and updated I also fixed the performance issue in 213fa5b |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request optimizes ReadRowsQuery and RowRange by introducing O(1) set-based deduplication for row keys and ranges, enforcing __slots__ to reduce memory allocation, and optimizing object copying and instantiation. The review feedback suggests simplifying the nested conditional logic in _signature_from_pb by using the protobuf WhichOneof method to improve readability and idiomatic handling of the oneof fields.
🤖 I have created a release *beep* *boop* --- ## [2.50.2](google-cloud-bigtable-v2.50.1...google-cloud-bigtable-v2.50.2) (2026-10-22) ### Bug Fixes * **bigtable:** preserve resource labels on batcher metrics ([#18712](#18712)) ([f18cc6b](f18cc6b)) * **bigtable:** resolve O(n^2) bottleneck in ReadRowsQuery.add_range ([#18706](#18706)) ([5ac7cc6](5ac7cc6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
This PR resolves a severe client-side O(n^2) CPU performance bottleneck inside
ReadRowsQuery.add_range. This regression was introduced when deduplication checks were added to prevent duplicate row ranges in read requests.Fixes b/573522284
Fixes #18708