Batch processing: fix maxBatches for partitioned data - #1786
Open
mzueva wants to merge 3 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: b309bd3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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
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.
MILAB-6799
Previous version incorrectly estimate total data size for partitioned data and this lead to wrong maxBatches calculation. This PR fixes the issue
Greptile Summary
The PR fixes batch-count enforcement and row preservation for partitioned PColumn data by moving the authoritative calculation into the split template, where the true per-scope row count is available.
batchCountfrom actual records for bothpassContentmodes, preventing partition-count estimates from dropping rows or bypassing the cap.maxBatchesbudget among isolation scopes and deterministically inflates each scope’s batch size.maxBatches— the soft upper bound on the total number of batch invocations; enforcement moves from an unreliable partition-based estimate to per-scope true counts.passContent— selects whether batch data is supplied directly as content or as a file reference; both paths now recompute batch counts from authoritative data.actualRecords— the true number of rows in one isolation scope; newly used to derive batch count and effective batch size.scopeMaxBatches— the newly introduced per-isolation-scope share ofmaxBatches, passed from the orchestrator to the split template.Confidence Score: 5/5
The PR appears safe to merge, with the changed batch-count paths preserving rows and enforcing the configured global cap.
The authoritative row count is now obtained inside each split scope, cap allocation remains globally bounded, and focused tests cover partitioned inputs along both direct-content and file-reference paths.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Partitioned or inline PColumn] --> B[Orchestrator groups isolation scopes] B --> C[Assign equal scopeMaxBatches budget] C --> D[Split template obtains actualRecords] D --> E{Computed batches exceed scope budget?} E -- Yes --> F[Inflate batchSize deterministically] E -- No --> G[Keep effective batchSize] F --> H[Slice and invoke batch body] G --> H H --> I[Merge batch outputs]Reviews (1): Last reviewed commit: "Fix max batch count for partitioned data" | Re-trigger Greptile
Context used: