[chore] Add sharded batcher wrapper for unpartitioned batching - #15460
[chore] Add sharded batcher wrapper for unpartitioned batching#15460iblancasa wants to merge 10 commits into
Conversation
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15460 +/- ##
=======================================
Coverage 91.04% 91.05%
=======================================
Files 728 729 +1
Lines 48467 48488 +21
=======================================
+ Hits 44129 44150 +21
Misses 3011 3011
Partials 1327 1327 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mwear
left a comment
There was a problem hiding this comment.
Mostly LGTM. I understand that the goal is to keep behavior unchanged by routing through a single shard. But the round-robin multi-shard routing is already implemented and reachable, and every test uses one shard, so that distribution path never actually runs. What do you think about a shardCount > 1 test that checks requests get spread across shards and that all shards flush and drain correctly?
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
|
@mwear I was planning to add this in follow up PRs but, at the same time, understand that it can be interesting to see this now. Added. |
|
@iblancasa can you please submit a follow-up PR on top of this in draft state? I'd like to see how you plan this to be implemented before merging this. |
Done #15534 |
Description
Adds an internal
shardedBatcherwrapper for the unpartitioned exporterhelper batch path. It keeps behavior unchanged by constructing the wrapper with exactly one shard. The shard itself is the existingpartitionBatcher, so current batching semantics formin_size,flush_timeout, shutdown draining, and request completion remain delegated to the existing implementation.This is a first step to allow unpartitioned batching to use multiple independent batch streams (follow up PR).
Link to tracking issue
Updates #12473
Testing
make testinexporterhelperAuthorship