feat(rfc): one tuning vocabulary — threads and fetch_size - #129
Merged
Conversation
Paging and parallelism were spelled differently in every extension: RFC had THREADS plus erpl_rfc_read_table_batch_budget, ODP had threads and no settings at all, BICS had neither. Learning one taught you nothing about the others. Settles the shared names and applies them to sap_read_table first: threads / erpl_rfc_max_threads -- how many SAP calls run at once fetch_size / erpl_rfc_fetch_size -- how much is asked for per round-trip fetch_size keeps each protocol's natural unit; here that is concurrent result rows, which is what bounds the SAP SDK's buffer on wide tables. It was previously reachable only as a session setting, so it could not be tuned per query. No breaking renames: erpl_rfc_read_table_batch_budget keeps working and writes the same value. They are two spellings of one knob, not two knobs. Both are transport settings, and the test asserts exactly that -- DD02L read at the default, at a fetch_size small enough to force many extra round-trips, and with threads=2, compared by symmetric EXCEPT ALL. If a batch boundary could drop or duplicate a row, that is where it would show.
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
1 participant
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.
Step 1 of the next plan block. Taken first because it constrains every later item — retrofitting consistency after three extensions have shipped divergent names is how the current divergence arose.
The problem
Paging and parallelism were spelled differently in each extension:
THREADS+erpl_rfc_read_table_batch_budgetthreads, and zero settings registeredLearning one taught you nothing about the others.
The vocabulary
threadserpl_<ext>_max_threadsfetch_sizeerpl_<ext>_fetch_sizefetch_sizekeeps each protocol's natural unit — forsap_read_tablethat is concurrent result rows, which is what bounds the SAP SDK's own buffer on wide tables. It was previously reachable only as a session setting, so it could not be tuned per query.This PR applies it to
sap_read_table. ODP follows in its own PR (it also gainsI_MAXPACKAGESIZE, currently hard-coded at 2 MiB and unreachable); BICS gainsthreadswhen chunking lands.No breaking renames
erpl_rfc_read_table_batch_budgetkeeps working and writes the same value. They are two spellings of one knob, not two knobs — documented as such.Testing
Both are transport settings, and the test asserts exactly that: DD02L (~165k rows) read at the default, at a
fetch_sizesmall enough to force many extra round-trips, and withthreads = 2— compared by symmetricEXCEPT ALL. If a batch boundary could drop or duplicate a row, that is where it would show.C++ suite green (39 cases, 2825 assertions).
Note on documentation
The
threadsdescription deliberately does not quote a recommended value. How much parallelism helps depends on the SAP system's capacity — work processes, application servers, database sessions — not on erpl, and a figure measured on a single-container trial would understate a real system. The docs say to raise it while watching throughput, and to check with Basis before a large parallel extract against production.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.