perf(preflight): batch IFoE L2 ping with ScriptLet [AIMVT-331] - #411
Merged
Merged
Conversation
…dispatches) IFoE L2 connectivity issued one cluster-wide dispatch per work item: port discovery cost 2 dispatches + 1 SFTP per (node, BDF), and every planned ping cost another dispatch via _exec_on_node(), which ran the real command on one host and `true` on all the others. At the production shape (4 nodes x 8 BDFs x 7 vPOD peers, ports='up') that is 321 serialized round trips. Both phases now follow the ScriptLet pattern already proven by rdma_connectivity: generate one script per node containing all of that node's work, distribute the scripts in a single parallel SCP, execute them with one exec_cmd_list, and harvest the results. Same shape, measured on a fake two/four-node cluster driving the real code path: 321 -> 21 dispatches, with byte-identical results to the unbatched path. Design notes where this deliberately differs from RDMA: * No `set -e`. RDMA can afford it because every test is backgrounded and reaped with `wait` (always 0). IFoE ping invocations are synchronous and their exit codes are load-bearing, so `set -e` would let the first failing ping abort the node's remaining tests and report them as *missing* rather than *failed* - a false PASS in a check that exists to find failures. * Per-invocation exit status is preserved, not discarded. Each invocation is framed by a BEGIN/END marker pair tagged with a per-run random token, and the END marker carries that invocation's own exit status. * Full stdout is preserved, not grepped. RDMA's collection scripts emit key=value failure summaries; IFoE must keep the complete output because the real verdict comes from AfmctlPingParser + _evaluate_summary + _evaluate_invocation (loss vs threshold, port coverage, pings-per-port echo, BDF echo, destination match). A `Summary:` grep would pass a run with 100% loss. `raw_output` and `parsed` are unchanged in the result structure. * Port inventory JSON still travels over SFTP, not stdout, per the explicit guidance on Pssh.download_file. Artifact paths are now stable per round (not uuid-per-call) so one download call serves every node holding a BDF; grouped downloads fall back to per-node retries so one node missing a device cannot mask the others. * Each invocation is individually bounded by coreutils `timeout` (falling back to unbounded if absent), so a hung afmctl cannot consume the whole batch window. Correctness properties kept: * Public result structure unchanged; batched and unbatched paths produce identical results (verified by differential test). * Commands run via `bash -c` so the `exit 127` embedded in the sudo command form cannot tear down the batch. * A missing result block fails loudly with an explicit error instead of silently vanishing from coverage. * Backends without real exec_cmd_list/upload_file_list (mocks, lightweight wrappers) keep the per-item path via _batching_supported(). * Any batch failure falls back to the per-item path. * Honors debug.scriptlet and reporting.artifacts_root_dir like RDMA. Also removes _all_unique_bdfs and _resolve_bdfs_for_node, which had zero callers repo-wide. Co-Authored-By: Claude <noreply@anthropic.com>
Pssh._process_output appends a host's stderr after its stdout, so any stderr output from the targeted afmctl command pushed the __CVS_AFMCTL_EXIT_STATUS__ sentinel out of the end-anchored regex match in _extract_exit_sentinel, causing exit_status to fall back to None. _evaluate_invocation treats None as non-failing, so a failing afmctl invocation that wrote anything to stderr was silently scored PASS. Search for the sentinel anywhere in the combined output instead of anchoring to the end, and splice out only its own line so trailing stderr content is preserved rather than discarded.
…gurable 10s check_connectivity()'s 2-second, zero-retry probe for a brand-new SSH connection is too tight for real hardware: establishing a fresh TCP+SSH session (especially right after a prior command kept the host busy) can easily exceed 2s, causing the "double check" to fail and the host to be permanently pruned from reachable_hosts on what was really a transient timeout. Observed on real hardware where the first successful invocation was immediately followed by false-positive pruning, silently turning all later invocations in the run into no-ops.
… a configurable 10s" This reverts commit d23a501.
cijohnson
approved these changes
Sep 11, 2026
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
2 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.
Summary
afmctlexit status when stderr is present (end-anchored sentinel was silently scored PASS).Test plan
test_ifoe_l2_connectivity.py(batched vs unbatched differential)main