Refactor env vars to clarify they are for individual ranks#449
Draft
scott-routledge2 wants to merge 4 commits intomainfrom
Draft
Refactor env vars to clarify they are for individual ranks#449scott-routledge2 wants to merge 4 commits intomainfrom
scott-routledge2 wants to merge 4 commits intomainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❌ Your project check has failed because the head coverage (66.42%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #449 +/- ##
==========================================
- Coverage 66.52% 66.42% -0.11%
==========================================
Files 176 176
Lines 63332 63491 +159
Branches 8875 8891 +16
==========================================
+ Hits 42133 42172 +39
- Misses 18574 18691 +117
- Partials 2625 2628 +3 |
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.
Changes included in this PR
See here for more context. Essentially the environment variables for the buffer pool are split between cluster-level, node-level and rank-level granularity, which can cause confusion.
For example:
BODO_BUFFER_POOL_MEMORY_SIZE_MiBspecifies the memory available to this rank for the buffer pool, whereasBODO_BUFFER_POOL_STORAGE_CONFIG_1_SPACE_PER_DRIVE_GiBIs the space available for each node (assuming one drive).Ideally IMO
BODO_BUFFER_POOL_MEMORY_SIZE_MiBshould just specify the amount of space available to all processes (on the same node) and then that space gets divided among ranks evenly, but then it creates a weird situation withBODO_BUFFER_POOL_MALLOC_FREE_TRIM_THRESHOLD_MiBstill being per rank.Adding "worker" makes it a bit more clear but then the environment variables too long e.g.
BODO_BUFFER_POOL_WORKER_MALLOC_FREE_TRIM_THRESHOLD_MiBTesting strategy
User facing changes
Checklist
[run CI]in your commit message.