Fix/hugepage doubling into r26.2 pre - #1302
Open
wmousa wants to merge 3 commits into
Open
Conversation
* fix: adjust hugepages bidirectionally (reduce when over-provisioned)
Previously hugepages were only increased when below the required amount.
Now the value is set whenever current != required, allowing reduction when
the node has more hugepages allocated than needed. Kubelet restart in the
storage init job is also triggered on any change, not just increases.
* fix: sync sn_config_file and hugepages when max_lvol changes on restart
When sn restart is called with a new max_lvol, the sn_config_file on the
node was not updated and hugepages were not adjusted to match the new
requirement.
- Add update_node_config API endpoint (docker + k8s) that updates
max_lvol and huge_page_memory for the matching NUMA node in
sn_config_file, matched by socket + SSD PCI intersection
- Add update_node_config client method in SNodeClient
- In _restart_storage_node_impl, call update_node_config before
set_hugepages() only when max_lvol actually changed (lvol_changed flag
captures the delta before snode.max_lvol is overwritten)
- Fix hugepage drift bug in set_hugepages_if_needed: store the adjusted
kernel total (required) instead of raw hugepages_needed so that
user_delta = current - prev correctly reflects only manual changes
* fix: persist user hugepage changes across restarts by updating baseline
When a user adds or removes hugepages manually between deploys, the delta
is captured correctly for the first restart but then silently lost on the
second: prev_sb advances to the new required (which already absorbed the
delta), so user_delta becomes 0 and the baseline wins � reverting to the
pre-change count.
Fix by writing the updated user reservation (baseline + delta) back to
hugepages_baseline_nodeN whenever user_delta != 0. The baseline then
reflects the user's intended permanent allocation, and each subsequent
restart computes the correct total without needing a manual re-add.
* fix: move hugepage state files from /tmp to /var/run/simplyblock
/tmp is subject to periodic cleanup by systemd-tmpfiles-clean (default
10-day TTL), which could silently delete hugepages_sb_node* and
hugepages_baseline_node* between deployments without a reboot, causing
the baseline tracking to reset incorrectly.
/var/run (/run) is a tmpfs that clears only on reboot � the correct
behaviour for these files � and is never touched by tmpfiles cleanup.
Updates the constant, docker volume mount, k8s hostPath mounts, and
the init job shell script.
* Deployment passed, fix env_var
* Fix comments from reviews
1. env_var � reverted image registry
Changed simplyblock/simplyblock:main and simplyblock/spdk:main-latest back to public.ecr.aws/simply-block/simplyblock:main and public.ecr.aws/simply-block/ultra:main-latest (matches origin/main).
2. Type annotations in UpdateNodeConfigParams (docker.py)
- ssd_list: list ? Optional[List[str]] = Field(None)
- max_lvol: int / huge_page_memory: int ? Optional[int] = Field(None, ge=0) (non-negative + partial update support)
- numa: int ? numa_node: Optional[int] = Field(None, ge=0)
- Endpoint logic updated to skip None fields and use numa_node
3. Deduplication (kubernetes.py)
Removed the entire duplicate UpdateNodeConfigParams class and 26-line endpoint body. The kubernetes endpoint now delegates directly to snode_ops.update_node_config(body) using snode_ops.UpdateNodeConfigParams as the body type (docker
is already imported as snode_ops). Net -22 lines.
4. Expose _save_sb_hugepages_allocation failures
Function now returns bool. The caller in set_hugepages_if_needed checks the return value and logs an error when persistence fails, so state inconsistency is surfaced rather than silently swallowed.
5. snode_client.py � parameter renamed from numa to numa_node to match.
* Rename update_node_config to persist_node_config
(cherry picked from commit df483b9)
set_hugepages_if_needed tracked manual user hugepage changes as user_delta = current_nr_hugepages - prev_sb, assuming nr_hugepages only changes by user action between deploys. A reboot (or any reset) zeroes the runtime allocation while the persisted prev_sb survives, so current(0) - prev_sb(7168) = -7168 was misread as "user removed 7168 hugepages". That negative delta was folded into the user baseline and persisted, producing a negative required value; `echo -512 | tee nr_hugepages` fails with "Invalid argument", the write raises before prev_sb is updated, and every retry repeats and compounds the baseline (0 -> -7168 -> -14336 -> ...), so hugepages are never set and the node never gets its SPDK pod (observed 2026-07-07 on worker-1). Treat user_delta <= 0 as a reset, not a removal: keep the established user baseline instead of going negative. Clamp the user baseline to >=0 and the written total to >= hugepages_needed so a negative value can never be computed or written. (cherry picked from commit e651264)
Covers the fault the two cherry-picked commits fix, plus the invariant that made it possible: every component that captures or reads the hugepage baseline must bind `_HUGEPAGES_BASELINE_DIR` to the same path on the host. - restart is a no-op; the requirement is never added to itself - the sb file records the total written to the kernel, not the raw requirement - a pre-existing user reservation is added once, and a manual addition made between restarts is neither reverted nor double-counted - a reboot re-applies the requirement without ever writing a negative value - a smaller requirement shrinks the reservation - the k8s init job / SPDK pod templates and the docker SNodeAPI volume list all host-mount the state dir, and the init job writes its baseline into it The operator's storage-node DaemonSet mount cannot be checked from this repo; it needs operator v26.2.8-1 or later.
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.
No description provided.