Add lblk cluster mode (Linux block devices via SPDK AIO bdevs) + campaign fixes - #1224
Open
schmidt-scaled wants to merge 9 commits into
Open
Add lblk cluster mode (Linux block devices via SPDK AIO bdevs) + campaign fixes#1224schmidt-scaled wants to merge 9 commits into
schmidt-scaled wants to merge 9 commits into
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| def test_reappearance_clears_counter(self): | ||
| dev = _aio_dev(serial="S1") | ||
| gone = self._node_with_inventory([dev], []) |
| try: | ||
| rpc_client.bdev_set_qd_sampling_period( | ||
| dev.nvme_bdev, constants.AIO_QD_SAMPLING_PERIOD_US) | ||
| except Exception: |
| container.id[:12], exc) | ||
| try: | ||
| container.remove(force=True) | ||
| except NotFound: |
| client.containers.get(name) | ||
| ok = False | ||
| logger.error("cleanup: %s still present after remove", name) | ||
| except NotFound: |
| base = f"/sys/block/{name}" | ||
| try: | ||
| holders.extend(os.listdir(f"{base}/holders")) | ||
| except OSError: |
| if entry.startswith(name): | ||
| try: | ||
| holders.extend(os.listdir(f"{base}/{entry}/holders")) | ||
| except OSError: |
| holders.extend(os.listdir(f"{base}/{entry}/holders")) | ||
| except OSError: | ||
| pass | ||
| except OSError: |
mxsrc
requested changes
Aug 12, 2026
mxsrc
left a comment
mxsrc
left a comment
Collaborator
There was a problem hiding this comment.
Looks largely good, most of my comments are very narrow. The one big thing is the device binding issue.
A few things that should be applied globally:
- avoid
except Exception: This hides programming errors, use specific exceptions if at all possible. E.g. for RPC interactionRPCExceptioncan be used. - imports should be at the top of the file. Otherwise modules are loaded at each execution of the function.
Comment on lines
+259
to
+267
| def _validated_device_mode(device_mode) -> str: | ||
| """Normalize/validate the cluster device mode ("nvme" | "lblk"). | ||
| Deploy-time only, like enable_failure_domain.""" | ||
| mode = (device_mode or constants.DEVICE_MODE_NVME).lower() | ||
| if mode not in (constants.DEVICE_MODE_NVME, constants.DEVICE_MODE_LBLK): | ||
| raise ValueError( | ||
| f"invalid device_mode {device_mode!r}; must be " | ||
| f"'{constants.DEVICE_MODE_NVME}' or '{constants.DEVICE_MODE_LBLK}'") | ||
| return mode |
Collaborator
There was a problem hiding this comment.
This can be a type annotation.
| by_name = {d["name"]: d for d in host_devices} | ||
| resolved, missing = [], [] | ||
| for entry in configured_entries: | ||
| live = by_serial.get(entry.get("serial")) or by_name.get(entry.get("name")) |
Collaborator
There was a problem hiding this comment.
This will resolve the wrong device just based on the slot. If a disk is changed in the same slot, the serial will not match, but the name will (e.g. sdb). The device will not be flagged as missing, and worse, it will silently use an unrelated device.
Suggested fix: Instead, if a serial is present in the device we're looking for, it must match, only if it is missing we fall back to name-based resolution, and we need to make sure that this is practically only used for initial discovery (i.e. we find it be name, and based on that determine the serial that will be used for future lookups).
Comment on lines
+1209
to
+1231
| def bdev_aio_create(self, name, filename, block_size=0): | ||
| """Create an SPDK AIO bdev over a Linux block device (lblk cluster | ||
| mode). ``filename`` is the device path — prefer the stable | ||
| /dev/disk/by-id symlink. ``block_size`` 0 lets SPDK use the device's | ||
| logical block size.""" | ||
| params = {"name": name, "filename": filename} | ||
| if block_size: | ||
| params["block_size"] = block_size | ||
| return self._request("bdev_aio_create", params) | ||
|
|
||
| def bdev_aio_delete(self, name): | ||
| return self._request("bdev_aio_delete", {"name": name}) | ||
|
|
||
| def bdev_aio_rescan(self, name): | ||
| """Re-read the backing device's size (device grow pickup).""" | ||
| return self._request("bdev_aio_rescan", {"name": name}) | ||
|
|
||
| def bdev_set_qd_sampling_period(self, name, period_us): | ||
| """Enable queue-depth sampling on a bdev so bdev_get_iostat reports | ||
| queue_depth/io_time — the hung-IO watchdog's signal for AIO base | ||
| bdevs (period 0 disables).""" | ||
| params = {"name": name, "period": period_us} | ||
| return self._request("bdev_set_qd_sampling_period", params) |
Collaborator
There was a problem hiding this comment.
These all silently ignore errors. They should instead use _request3, which will raise appropriate errors.
Comment on lines
+1233
to
+1237
| def get_bdevs_2(self, name): | ||
| """(ret, err) probe variant of bdev_get_bdevs, mirroring | ||
| bdev_nvme_controller_list_2 — used where the caller must distinguish | ||
| 'bdev gone' from RPC failure without raising.""" | ||
| return self._request2("bdev_get_bdevs", {"name": name}) |
Collaborator
There was a problem hiding this comment.
This deviates form the original: The warning is not present, its semantics are hidden (e.g. explicitly passing None gives the full list, and in the regular use-case (passing a name) its name lies, it does only return one device.
I suggest to instead adapt RPCClient.get_bdevs to also use _request3, which will silently upgrade it to handle errors, with the added advantage of more robust error reporting elsewhere. This still has the naming issue, but it's well-known in the code base so it can be adapted at a later point.
Comment on lines
+1568
to
+1588
| # JM mesh gate (2026-08-05 incident: nodes joined via add-node retries | ||
| # activated with peers missing their remote_jm controllers — the cluster | ||
| # reported healthy while a third of the journal mesh was unreachable, | ||
| # and the first journal load collapsed n_safe_jms into a cluster-wide | ||
| # JCERR). FRESH activation must not complete over such a hole; a | ||
| # RE-ACTIVATION is a recovery path that may legitimately run with one | ||
| # or two nodes unhealthy, so it repairs best-effort and only warns — | ||
| # the verifier already skips JMs whose owner node is not ONLINE. | ||
| if cluster.ha_type == "ha": | ||
| jm_problems = storage_node_ops.verify_jm_mesh_coverage(cl_id, repair=True) | ||
| if jm_problems: | ||
| if is_fresh_activation: | ||
| set_cluster_status(cl_id, ols_status) | ||
| raise ValueError( | ||
| "Failed to activate cluster: JM mesh coverage incomplete " | ||
| "(journal quorum would silently run degraded): " | ||
| + "; ".join(jm_problems)) | ||
| logger.warning( | ||
| "JM mesh coverage incomplete on re-activation (continuing — " | ||
| "recovery path): %s", "; ".join(jm_problems)) | ||
|
|
Collaborator
There was a problem hiding this comment.
This runs after lvstore creation, QoS, JC-compression, and MCP narrowing. A failure here only rolls back the cluster status but leaves the other state in place.
| def _disk_holders(name: str) -> List[str]: | ||
| """Union of /sys/block/<d>/holders and every partition's holders — | ||
| catches LVM PVs, md members and dm-crypt without a mountpoint.""" | ||
| import os |
| return [] | ||
|
|
||
|
|
||
| def _read_sysfs(path: str) -> str: |
Collaborator
There was a problem hiding this comment.
This should return an Optional[str] with None in the failure case.
| def _disk_by_id_path(name: str) -> str: | ||
| """Preferred stable /dev/disk/by-id symlink for a whole disk: wwn-* first, | ||
| then any other non-partition link. Empty when none exists.""" | ||
| import os |
Collaborator
There was a problem hiding this comment.
Runtime imports should be avoided, this is better placed at the top.
schmidt-scaled
force-pushed
the
md-journal
branch
from
August 14, 2026 10:10
8385dd9 to
e64b2ff
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def ssh_exec(ip, cmds, get_output=False, check=False, timeout=900): | ||
| ssh = paramiko.SSHClient() | ||
| ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) |
| serial is appended so distinct serials can never collide.""" | ||
| sanitized = re.sub(r"[^A-Za-z0-9_]", "_", serial) | ||
| if sanitized != serial or len(sanitized) > 40: | ||
| digest = hashlib.sha1(serial.encode()).hexdigest()[:6] |
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) |
| try: | ||
| if os.path.realpath(path) == f"/dev/{name}": | ||
| return path | ||
| except OSError: |
New cluster-global device mode chosen at cluster create (--device-mode
nvme|lblk, deploy-time only). In lblk mode, eligible Linux block devices
(unmounted, unheld, unpartitioned whole disks; partitioned only with
--force-format on add-node, which wipes them host-side) are wrapped in
one SPDK AIO bdev per device. Everything from alceml upward is unchanged;
nvme-tcp/rdma fabric is untouched.
- Selection at `sn configure --lblk` (and the k8s node_configure twin) by
device name include/exclude or serial number; the node config file
carries an editable `lblk_devices` list ({name, serial, by_id, size,
numa}) parallel to ssd_pcis, validated as exactly-one-device-source.
- Identity is SERIAL-FIRST (lsblk SERIAL -> WWN -> stable synthetic id):
add-node persists the selection on the node record; restart re-resolves
serial -> current kernel name (stored name as fallback), so device
renames across reboots cannot attach the wrong disk. AIO bdev names are
derived from the serial (collision-safe) and stable across restarts.
The serial-keyed restart reconcile works unchanged; missing device ->
STATUS_REMOVED, new -> STATUS_NEW, same as nvme.
- SPDK launch in lblk mode never passes an empty PCI allowlist (DPDK
treats empty as allow-all; the k8s path passes PCI_ALLOWED="" today) —
a host-bridge placeholder 0000:00:00.0 is used instead, and no
vfio/uio binds ever happen, so SPDK cannot claim kernel disks.
- Failure parity (control plane only): the distrib error_* event path is
already bdev-generic; the hung-IO gap (AIO has no bdev_nvme timeout_us/
action_on_timeout) is closed by a device_monitor watchdog using
queue-depth-sampled iostat — inflight IO with zero completion progress
across 3 polls (30s) feeds io_error + UNAVAILABLE with a countable
LOCAL_FAILURE cause into the existing flap/auto-restart/FAILED/
migration machinery; >=2 simultaneously stalled devices escalate to a
node auto-restart; RPC failures freeze (never advance) the counters.
Device disappearance from the host inventory drives device_remove (the
SPDK_BDEV_EVENT_REMOVE treatment) after a 2-poll debounce. The
late-event gate, reset (liveness probe — never delete/recreate the aio
bdev in place), SMART info, restart_device and new_device_from_failed
are mode-aware.
- New snode endpoints: GET /blockdevices (whole-disk inventory with
eligibility fields, by-id path, NUMA) and POST /wipe_block_device
(re-validates busy state, wipefs partitions-then-disk), on both docker
and k8s agents.
- Phase-1 scope: lblk requires journal-on-device (GPT-partition JM mode
is nvme-only); --ssd-pcie/--reattach-volume are rejected on lblk nodes.
tests: 93 unit tests (eligibility/onboarding/watchdog/device_controller)
+ 10 FDB-backed integration tests (model round-trips, restart identity
contract over renamed devices, watchdog -> real state machine, flap-limit
force-FAILED + migration, disappearance -> device_remove, reset). Full
unit tier 1005 green, ruff clean.
External follow-ups: validate run_distr_with_ssd.sh tolerates the
placeholder -A on a test node; confirm the fork's bdev_get_iostat carries
qd-sampling fields; alceml over 512e aio bdevs (block_size currently
omitted, 4096 fallback if needed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first AWS lblk deploy (2x50G EBS per 32 GiB host) failed every `sn configure --lblk` with "Free memory ... less than required 127228418457": node_config_min_sys_memory charged 2 GiB + the FULL device capacity. The nvme path nominally does the same but always measures zero — capacity is read via `nvme list` after the devices were unbound from the kernel driver — so the de-facto contract (and the documented intent, "plus 0.2% of the storage") is a small fraction. Apply the documented 0.2% factor for lblk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gate Two control-plane gaps exposed by the 2026-08-05 lblk soak bring-up (both generic, neither lblk-specific): 1. Zombie SPDK on failure cleanup. When add-node/restart aborts with "node did not come up", _kill_spdk_until_dead verified death via spdk_process_is_up — an RPC-Unix-socket probe that false-negatives an SPDK which booted but never brought its RPC up. Combined with spdk_process_kill's deliberately detached container remove (fast peer termination) losing the race against the container restart policy, the "confirmed down" SPDK survived, squatting ~all hugepages and starving every subsequent add-node retry on the host. New agent endpoint spdk_process_cleanup: clears the restart policy, removes synchronously, and reports success only when the containers are verifiably GONE (k8s agent: alias of its already-synchronous pod-delete-and-poll). _kill_spdk_until_dead prefers it and falls back to the legacy kill + socket-poll for older agents. 2. JM-mesh activation gate. Nodes that joined through add-node retries ended with peers missing their remote_jm_* controllers; the cluster activated and reported healthy while a third of the journal mesh was unreachable. First journal load excluded those JMs, n_safe_jms collapsed and JCERR cascaded cluster-wide. New storage_node_ops.verify_jm_mesh_coverage(): every ONLINE node must hold live remote bdevs for the remote JMs it references, with a one-shot _connect_to_remote_jm_devs repair. Wired into _cluster_activate: FRESH activation fails on unrepaired holes; RE-ACTIVATION is a recovery path that may legitimately run with one or two nodes unhealthy — the verifier skips JMs whose owner is not ONLINE and the gate only warns. tests: tests/unit/test_jm_mesh_and_spdk_cleanup.py (11 cases); full unit tier 1000 green, ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
md-journal test campaigns now run against the fork's md-journal branch (blobstore metadata journal work) instead of ultra main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ol ops 1-node clusters deploy without HA journaling and activate as non-HA regardless of the chosen ha_type / EC schema: - add_node forces enable_ha_jm=False on is_single_node clusters (deterministic jm_vuid=1 / LVS_1 single local journal, no fabric export). - Activation treats a cluster with exactly one online node as non-HA: role assignment and the JM-mesh gate are skipped even with ha_type=ha, fresh nodes are forced to the single-journal shape, and the +1 spare-device requirement is dropped (a 2-unit node can activate EC 1+0). - Physical labels stay unused: the activation label rewrite now also re-syncs the per-device label copies the distrib cluster map reads. - Lvol lifecycle ops run on exactly one node: HA lvol creation downgrades to ha_type=single on hosts without a secondary (the unguarded secondary lookup previously 500'd on 2+-node non-HA clusters and only worked by accident on 1-node ones); empty role ids are never emitted into lvol.nodes; snapshot phase-2 sync deletes and snapshot health checks are gated on the lvol's ha_type instead of node topology. 47 new unit tests in test_single_node_cluster.py (shared with the lblk partition tests); unit tier 1310 green.
…partition split Nodes can now run on partitions instead of full SSDs only (lblk mode): - Inventory (/blockdevices) emits partitions with stable identity: serial derived from the parent disk serial + PARTUUID, by-partuuid stable path, per-partition busy/holder/root detection. Restart resolution falls back to the PARTUUID when the parent serial changed. - Eligibility: partitions must be unmounted (not busy), unheld, non-root, writable; they are never auto-selected (explicit --blk-names/--blk-serials only), and a disk and its own partitions cannot both be selected. - Journal carve-out: partition-backed selections never relabel a whole drive - the smallest selected partition is SPLIT in two at configure time (sgdisk, GPT only, within the original partition bounds): a journal partition (--jm-percent of total capacity, 2 GiB floor, capped at half the split partition) and a data partition in the remainder. The journal entry is flagged in the node config; add-node/restart prefer it over smallest-device selection. - Minimum 2 partitions or SSDs per node, enforced at configure, add-node and config validation (plus at most one journal-flagged entry). - validate_arguments in node_configure tolerates minimal namespaces (pre-existing rebase fallout with main's max-subsystems test). New unit tests: test_lblk_partitions.py (inventory, split orchestration + mechanics, PARTUUID resolution, flagged-journal selection); eligibility tests extended for partition semantics. Unit tier 1310 green, ruff clean.
…rt / 4 part) deploy_single_node_lblk.py deploys a 1-node non-HA lblk cluster on AWS in one of three device configurations: two whole EBS volumes (journal-on- device, EC 1+0), one volume with 2 GPT partitions (configure-time journal split, EC 1+1), or one volume with 4 partitions (EC 2+1). Self-contained (boto3+paramiko), emits cluster metadata JSON for the soak driver. single_node_partition_soak.py drives all three configs end-to-end: lvol create + nvme-tcp connect on the mgmt instance, crc32c-stamped data region plus mixed random IO, graceful `sn restart`, then a verify-only crc32c pass over the stamped region proving the data is available and uncorrupted after the node restart. Fleets are terminated on success and kept for debugging on failure.
… in soak cluster activate read records[0]['size_total'] unguarded. On a freshly deployed cluster the capacity collector has not necessarily run yet — a single-node deployment reaches activation seconds after add-node — so the read raised a bare "list index out of range" and activation aborted. Fall back to the raw sum of the online data devices (create_lvstore takes max_size but sizes its distribs from DISTRIB_SIZE_BYTES; the value only feeds the reported cluster_max_size). Soak scripts, from the first AWS run: - pin SIMPLY_BLOCK_DOCKER_IMAGE on cluster create / sn deploy / sn add-node, resolved from the repo HEAD and verified on ECR. A control-plane stack from an older image DROPS unknown model fields on read-modify-write, so device_mode=lblk silently reverted to nvme and add-node then refused the lblk node config. - fio verify pass must replay the write job with --verify_only (fio then skips the writes and only reads back); the previous --rw=read pass would have verified nothing. - install from github.com/simplyblock/sbcli (the old org name only resolves through a redirect).
get_next_cluster_status treated "affected_nodes == distr_npcs" as being at the parity limit. With npcs=0 (EC 1+0, the natural single-node schema) k is 0, so the condition matched on affected_nodes == 0 — a perfectly healthy cluster reported DEGRADED forever. Observed on the live 1-node soak cluster 2026-08-14: single data device online, JM online, node health True, cluster DEGRADED. Require at least one affected node before the limit rule applies; the npcs>=1 paths are unchanged (the pre-existing failures in tests/integration/test_cluster_suspend_recovery.py are identical with and without this change). Also make the soak's restart wait case-insensitive: `sn list` prints the node status lowercase but `cluster list` prints ACTIVE, so the wait spun until timeout on restarts that had actually succeeded.
schmidt-scaled
force-pushed
the
md-journal
branch
from
August 14, 2026 19:22
9d8da62 to
0640f32
Compare
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
4 participants
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.
lblk cluster mode: Linux block devices as storage via SPDK AIO bdevs
Rebased onto current main (152613f); the restart-claim fix formerly carried on this branch is already on main (17ebddb) and dropped out of the diff. Four commits:
1. lblk cluster mode (7757cd2)
Cluster-global
device_mode: nvme|lblkchosen atcluster create. Inlblkmode, eligible Linux block devices (whole, unmounted, unheld, unpartitioned disks — partitioned only with--force/--force-format) are wrapped in SPDK AIO bdevs; everything from alceml upward is unchanged, and the inter-node fabric (nvme-tcp/rdma) is untouched.sn configure --lblkby--blk-names/--blk-names-exclude/--blk-serials; config file manually editable.aio_<sanitized_serial>is stable.bdev_nvmeIO timeout) is closed by a hung-IO watchdog in DeviceMonitor (qd-sampling + iostat progress, 3×10 s → UNAVAILABLE; ≥2 stalled devices → node auto-restart); device disappearance handled via /blockdevices inventory sweeps; late-event gate, reset, restart, and new-device-from-failed all have mode-aware branches.0000:00:00.0(never an empty list — empty means DPDK allow-all on k8s).num_partitions_per_dev == 0);--ssd-pcierestart-time growth is rejected on lblk clusters.2. sys-memory sizing fix (5541736)
sn configure --lblkdemanded 2 GiB + full device capacity (102 GiB for a 100 GiB node). Now 2 GiB + 0.2 % of capacity. The nvme path accidentally measured zero (capacity read after driver unbind) and is unified on the same formula.3. Zombie-SPDK cleanup + JM-mesh activation gate (6e7fcb2)
Two field findings from the lblk soak campaigns, both mode-independent CP bugs:
spdk_process_cleanup(clears restart policy, synchronous verified remove);_kill_spdk_until_deadprefers it with legacy fallback.remote_jm_*controllers, silently degrading JC quartets until the first journal load collapsed them. Newverify_jm_mesh_coverage+ activation gate: fresh activation hard-fails on unrepaired mesh holes; re-activation only warns (recovery with unhealthy nodes must never be blocked). 11 unit tests.4. env_var: SPDK fork image →
ultra:md-journal-latest(8385dd9)Points the default fork image at the md-journal branch (blobstore metadata journal, simplyblock/spdk#61). Flagged for reviewer decision: merging this changes the default SPDK image for every deployment from this branch — drop this commit if main should stay on
ultra:main-latestuntil spdk#61 lands.Validation
constantsimportdevice_controller.pynow needs — fixed in the feature commit).ultra:md-journal-latest: full 60-scenario dual-node-outage churn soak (graceful / forced / container_kill / host_reboot / 65 s network partitions × unrelated / P-T / P-S pairs) — 60/60 passed, zero fio faults, zero cluster suspensions.🤖 Generated with Claude Code