Make AOSS insert batch size configurable - #843
Conversation
Signed-off-by: norrishuang <12380647@qq.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: norrishuang The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| ) -> tuple[int, Exception]: | ||
| embeddings_list = list(embeddings) | ||
| batch_size = 100 if self._is_serverless else len(embeddings_list) | ||
| batch_size = config.NUM_PER_BATCH if self._is_serverless else len(embeddings_list) |
There was a problem hiding this comment.
vectordb_bench/backend/clients/aws_opensearch/aws_opensearch.py line:260
Medium ---- NUM_PER_BATCH comes directly from an environment variable without a positivity check. If it is set to 0, range(..., batch_size) raises ValueError: range() arg 3 must not be zero before any request; a negative value makes the range empty and returns (0, None), falsely reporting a successful load. Please validate this setting (or clamp/reject non-positive values) before using it so operator misconfiguration cannot crash or silently skip ingestion.
There was a problem hiding this comment.
Fixed in fc5b23e. OpenSearch Serverless now rejects non-positive NUM_PER_BATCH values with a clear ValueError before constructing the range, and tests cover both zero and negative values.
|
Hi, thank you for the contribution! FYI we have a broader change to make num per batch a CLI variable coming in #813. Once this pr is merged, 813 will reflect upon this by controlling batch behavior through CLI. |
|
/lgtm |
Merging zilliztech#843 reintroduced config.NUM_PER_BATCH, which the insert-batch-size control feature removed. Route the AOSS serverless bulk chunk size through the task-level insert_batch_size instead, keeping the default of 100 and the non-positive guard. Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
4 participants
What changed
NUM_PER_BATCHconfiguration for OpenSearch Serverless bulk requests instead of a hard-coded batch size of 100.Testing