Conversation
The Bigtable online store hardcodes the mutations-per-write batch size (MUTATIONS_PER_OP = 50_000) and the write thread-pool size (BIGTABLE_CLIENT_CONNECTION_POOL_SIZE = 10). On a shared Bigtable instance, a large materialization issues its writes as an unthrottled burst that can saturate the instance and inflate read-path tail latency for other workloads sharing it. Expose both as optional BigtableOnlineStoreConfig fields, mutations_per_write and write_concurrency, defaulting to the existing constants so behavior is unchanged. Operators can lower either to reduce the write load a materialization places on the instance, at the cost of longer materialization time. Add unit tests covering the defaults, positive-int validation, batch chunking, the one-row-per-request floor for very wide feature views, and the configurable thread-pool size. Signed-off-by: Manas Bhardwaj <manas1109bhardwaj@gmail.com>
Author
|
Hi @franciscojavierarceo please review this. At our org, we want to control the number of writes and concurrency. As of now it is hard coded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
The Bigtable online store hardcodes two values that govern its write path:
MUTATIONS_PER_OP = 50_000— target mutations perMutateRowsrequestBIGTABLE_CLIENT_CONNECTION_POOL_SIZE = 10— theThreadPoolExecutorsize used to parallelize writes inonline_write_batchOn a shared Bigtable instance this is a problem: a large materialization fans its writes out across the thread pool with no way to tune the request size or concurrency, issuing an unthrottled write burst that can saturate the instance and inflate read-path tail latency for other workloads sharing it. Today the only way to soften that burst is to fork the online store.
This PR exposes both as optional
BigtableOnlineStoreConfigfields:mutations_per_write50000write_concurrency10Both default to the existing module constants, so behavior is unchanged unless explicitly configured. Operators running against a shared instance can now lower either value to reduce the write load a materialization places on Bigtable, trading materialization speed for lower peak write pressure. Both are validated as
PositiveInt, andonline_write_batchnow floors rows-per-request at 1 so a very wide feature view combined with a smallmutations_per_writecan't produce a zero-sized batch.Example:
Which issue(s) this PR fixes
N/A — backward-compatible enhancement.
Misc
sdk/python/tests/unit/infra/online_store/test_bigtable_online_store.pycovering defaults matching the legacy constants, positive-int validation, batch chunking, the wide-feature-view floor, and the configurable thread-pool size.