Skip to content

feat(posix): Asynchronous Open in POSIX-uring - #2200

Draft
lluki wants to merge 3 commits into
ai-dynamo:mainfrom
lluki:uring-fixed-files
Draft

lluki wants to merge 3 commits into
ai-dynamo:mainfrom
lluki:uring-fixed-files

Conversation

@lluki

@lluki lluki commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What?

Post open asynchronously to uring.

Why?

+50% perf when used the SgLang HiCache NIXL backend on NFS-over-RDMA

How?

1st commit: Refactor the posix_backend.cpp such that the io_engine always gets a devId. It's now the io engines job to do the devid -> fd/state mapping. Why? If not we need an awkward dance in posix_backend.cpp to find the right fd/devid to be passed in enqueue (note in the lazy open, the FD might not exist yet, so fd makes no sense anymore at that point)

2nd commit: implement the asynchronous mode in uring engine. The current implementation still bounces to userspace after open, but each open->read/write dependency is handled individually. It gets the same performance (at the slight cost of added CPU utilization) but it is compatible with kernel 5.15. If we drop support for 5.15, we can implement the linked SQEs.

State Machine Diagrams

Code is somewhat complex. There are two state machines: One for the file open/closed state, and one per transfer.

File State Diagram

  • Unreachable states are not displayed (like pendingClose without deregistered)
  • The posix backend drives the state machine using the following methods:
    • registerFile() called by nixlPosixEngine::registerMem(). It creates the FileState in PendingOpen and calls post().
    • deregisterFile() called by nixlPosixEngine::deregisterMem(). It invokes retireFile() and waits for closing to complete.
    • post() called by the common transfer path and internally. For file states, it submits pending opens/closes and erases closed entries.
    • poll() called by checkXfer(). It processes CQEs through doCheckCompleted() and then calls post().
file

Transfer State Diagram

  • The posix backend drives the transfer state machine through:
    • enqueue() from nixlPosixBackendReqH::postXfer(). It delegates to the internal enqueueIO(), which acquires and initializes the slot as Queued.
    • post() from postXfer(). It submits queued READ/WRITE operations. It is also called internally during cancellation and polling.
    • poll() from checkXfer(). It dispatches data/cancel completions and subsequently calls post().
    • cancel() from requestCancellation() after a transfer failure.
  • Unchanged from the current implementation, it signals per descriptor completion or cancellation using one of the two callbacks
transfer

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

👋 Hi lluki! Thank you for contributing to ai-dynamo/nixl.

Your PR reviewers will review your contribution then trigger the CI to test your changes.

🚀

@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentAWS NIXL Validation · commit 3bbc5a6b

TL;DR: The AWS Batch pod failed at git clone https://github.com/ai-dynamo/nixl with could not read Username for 'https://github.com': No such device or address, killing the job in 12s before any test ran. The clone is anonymous and then tries to git checkout pull-request/2200 — a synthetic CI ref that doesn't exist on public github.com — so the pod cannot fetch the PR code; fix by passing an auth token and pushing/fetching the actual commit SHA into the pod rather than the internal pull-request/* ref.

Full analysis

Summary: The AWS EFA validation job's in-pod git clone/checkout step failed immediately, so the AWS Batch job reached terminal status FAILED after 12s and the GHA step exited 1.

Root cause: contrib/aws-efa/aws_test.sh builds an in-pod command git clone https://github.com/ai-dynamo/nixl && cd nixl && git checkout pull-request/2200 && ... (see AWS_CMD, line 58). The pod (aws_vars.template) is given no GitHub credentials, so the clone runs anonymously and git fell back to an interactive username prompt — failing with could not read Username for 'https://github.com': No such device or address / fatal: expected flush after ref listing. Compounding this, the checkout target pull-request/2200 is the internal CI mirror's synthetic PR ref (the GHA checkout created origin/pull-request/2200 from commit 3bbc5a6b…), which does not exist on the public ai-dynamo/nixl remote the pod clones — so even a successful anonymous clone could not check out that ref. No NIXL code was built or tested; the failure is CI plumbing, not a defect in PR #2200.

Implicated commit: 2709adcd (ovidiusm, "Unify AWS test execution (#1434)") introduced the anonymous-clone-then-checkout-ref pattern in AWS_CMD; unchanged since. No commit in PR #2200 is implicated.

File: contrib/aws-efa/aws_test.sh:58 (the AWS_CMD clone/checkout), with the credential-less pod env in contrib/aws-efa/aws_vars.template:5-9.

Suggested fix: Make the pod able to fetch the PR code: (1) inject a token so the clone authenticates, e.g. clone https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY} and add -c credential.helper= / set GIT_TERMINAL_PROMPT=0 so a failed auth errors clearly instead of hanging on a prompt; and (2) check out the concrete commit SHA ([REDACTED:Hex High Entropy String]) via git fetch origin <sha> && git checkout <sha> instead of the internal pull-request/2200 ref, which does not exist on the public remote. If this is instead transient GitHub unavailability, add retry/backoff around the clone — but the missing-ref problem must be fixed regardless.

Related: none (PR #1912 surfaced by search is unrelated).

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 9048b771-f659-4c10-a324-f324cb560701 in the triage console for the audit trail.

@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentPR Size Check · commit d7dd5587

TL;DR: The "PR Size Check" workflow failed because PR #2200 modifies 851 lines in existing (non-subproject) files, exceeding the 500-line limit; either split the PR or the check needs an exemption/higher limit for this change.

Full analysis

Summary: The check-pr-size job in .github/workflows/pr-size-check.yml failed with exit code 1 — the PR exceeds the enforced 500-line change limit.

Root cause: This is an intentional CI policy gate, not a bug. The workflow computes lines added to modified files (excluding subprojects/*) via git show --numstat --pretty="" --diff-filter=M -- . ':(exclude)subprojects/*' and fails when the sum exceeds 500. For this PR the sum is 851, so the if [[ "$LINES_CHANGED" -gt 500 ]] branch ran exit 1. The build itself is healthy — checkout and everything else succeeded; the check simply reported the PR is too large.

Implicated commit: No faulty commit — the PR head is [REDACTED:Hex High Entropy String] (branch uring-fixed-files), merged as d866e85. The size is the aggregate of the PR's changes, not a single defect.

File: .github/workflows/pr-size-check.yml (the LINES_CHANGED / -gt 500 gate)

Suggested fix: This is a code-review/process issue, not a CI infrastructure fix. Options for the PR author:

  • Split PR feat(posix): Asynchronous Open in POSIX-uring #2200 into smaller PRs each under 500 changed lines (preferred, and what the gate is designed to encourage).
  • If the 851 lines are unavoidable and legitimate (e.g. large generated/vendored files that should be excluded), add the appropriate path to the :(exclude)... pathspec in the workflow, or obtain a maintainer override/label to bypass the check.
  • Do not raise the 500 limit purely to make this PR pass unless the team explicitly decides to change the repo-wide policy.

Related: none

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 43fe8de0-2da4-4d3f-ba0b-135ccbee09c5 in the triage console for the audit trail.

@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentClang Format Check · commit d7dd5587

TL;DR: The "Clang Format Check" job failed because several modified POSIX-plugin files in PR #2200 are not formatted per the repo's clang-format-19 style; run clang-format-19 -style=file -i on the flagged files and commit the result.

Full analysis

Summary: The clang-format GitHub Actions job exited with code 1 because clang-format-diff-19 reported formatting diffs in the C/C++ files changed by the PR.

Root cause: Code style violations, not a build/test defect. The workflow computes changed C/C++ files and pipes their diff through clang-format-diff-19 -p1 -style=file. clang-format-19 wants different formatting than what was committed, so it emitted a non-empty diff and the step failed. Concrete violations flagged:

  • src/plugins/posix/io_queue.cpp — multi-line enqueueFd(...) call should be a single line.
  • src/plugins/posix/io_queue.h — missing blank lines before struct RegisteredFile / files_.
  • src/plugins/posix/io_uring_io_queue.cppIO() ctor, completeQueuedIO decl, several if/call wrappings need reflowing; stray blank line to remove.
  • src/plugins/posix/linux_aio_io_queue.cpp & posix_aio_io_queue.cpp — misaligned continuation-line indentation of enqueueFd parameters.
  • src/plugins/posix/posix_backend.h — collapse nixlPosixFileMD ctor to one line; ~nixlPosixBackendReqH(){}~nixlPosixBackendReqH() {}.
  • test/unit/plugins/posix/nixl_posix_uring_path_agent_test.cpp and nixl_posix_uring_test.cpp — expression/createBackend/enqueue call reflows.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #2200 head, branch uring-fixed-files; merge commit d866e85). Author not shown in the log.

File: src/plugins/posix/io_queue.cpp:93 (plus the other files/hunks listed above); check runner from .github/workflows/clang-format.yml.

Suggested fix: Locally run clang-format-19 over the changed files and commit, e.g.:

clang-format-19 -style=file -i \
  src/plugins/posix/io_queue.cpp src/plugins/posix/io_queue.h \
  src/plugins/posix/io_uring_io_queue.cpp \
  src/plugins/posix/linux_aio_io_queue.cpp src/plugins/posix/posix_aio_io_queue.cpp \
  src/plugins/posix/posix_backend.cpp src/plugins/posix/posix_backend.h \
  test/unit/plugins/posix/nixl_posix_uring_path_agent_test.cpp \
  test/unit/plugins/posix/nixl_posix_uring_test.cpp

Then push the reformatted code. (Ensure the local clang-format is version 19 to match CI, since formatting differs across versions.)

Related: none found.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id cce28abd-0015-4a55-9b59-4aaf42d4cbc8 in the triage console for the audit trail.

@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentAWS NIXL Validation · commit d7dd5587

TL;DR: The AWS Batch pod's git clone https://github.com/ai-dynamo/nixl failed with could not read Username for 'https://github.com', causing the batch job to go FAILED; the in-pod clone runs with no credentials and cannot fetch the PR ref pull-request/2200, so the fix is to pass an auth token (or use the outer checkout instead of re-cloning) and clone the actual PR ref rather than a non-existent branch.

Full analysis

Summary: The "Run AWS Tests" GHA job submitted an AWS Batch job that failed inside the pod at the git clone step, so wait_for_status SUCCEEDED returned FAILED and the step exited 1.

Root cause: Inside the AWS Batch pod the command is git clone https://github.com/ai-dynamo/nixl && cd nixl && git checkout pull-request/2200. This clone runs with no git credentials — unlike the GH runner's actions/checkout, which injected an http.extraheader authorization. Git prompted for a username (fatal: could not read Username for 'https://github.com': No such device or address / fatal: expected flush after ref listing), the clone aborted, and the batch job immediately reached terminal FAILED (Job reached terminal status FAILED after 1s). The pull-request/2200 ref is a CI-synthesized PR ref that also is not present on the public remote for an anonymous clone/checkout, compounding the problem. This is an infrastructure/credential issue in the AWS test harness, not a defect in the PR's POSIX code.

Implicated commit: unknown for the failure trigger (the clone logic in contrib/aws-efa/aws_test.sh line 58 is long-standing, last touched by f4b10ff / 2709adc); the failure is environmental/auth-related rather than introduced by a specific commit.

File: contrib/aws-efa/aws_test.sh:58 (the AWS_CMD clone/checkout string)

Suggested fix: Make the in-pod clone authenticated and target the actual PR ref rather than a branch name. Concretely: (a) plumb a GitHub token into the AWS Batch pod env and use it in the clone URL / http.extraheader, and (b) fetch the PR ref explicitly, e.g. git clone $GITHUB_SERVER_URL/$GITHUB_REPOSITORY && cd nixl && git fetch origin "$GITHUB_SHA" && git checkout "$GITHUB_SHA" (using the commit SHA d7dd558... which is always reachable) instead of git checkout pull-request/2200. Also consider passing the workspace checkout into the job instead of re-cloning. Confirm the repo is anonymously cloneable from the batch node's network (the auth prompt suggests it currently is not).

Related: PR #2200 (the PR under test); no prior issue reporting this exact clone failure.

@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentPR Size Check · commit 997dbe64

TL;DR: The "PR Size Check" failed because PR #2200 modifies 879 lines in existing files (excluding subprojects/), exceeding the workflow's hard limit of 500. The fix is to split the PR into smaller pieces (or, if justified, adjust the limit in the workflow).

Full analysis

Summary: The check-pr-size job in .github/workflows/pr-size-check.yml exited 1 because the PR's modified-line count exceeds the 500-line threshold.

Root cause: This is a working-as-intended policy gate, not a code defect. The workflow step computes lines added to modified existing files via git show --numstat --pretty="" --diff-filter=M -- . ':(exclude)subprojects/*' and errors out when the total is > 500. For this PR the total is 879, so the guard printed "❌ PR size check failed! This PR adds 879 lines (excluding subprojects), which exceeds the maximum of 500 lines." and exited 1. There is no infrastructure or build error — checkout, fetch, and the script all ran normally in ~2 seconds.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #2200 head, branch uring-fixed-files; merge commit 036aff1). No specific author-blame beyond the PR content itself.

File: .github/workflows/pr-size-check.yml (the check-pr-size step; threshold at the if [[ "$LINES_CHANGED" -gt 500 ]] line).

Suggested fix: This is a size-policy failure, so the actionable fix is on the PR, not the CI:

  • Split PR feat(posix): Asynchronous Open in POSIX-uring #2200 into smaller, reviewable PRs so each stays under the 500-line limit, or
  • If the large change is unavoidable and pre-approved by maintainers, either raise the -gt 500 threshold in .github/workflows/pr-size-check.yml, or move genuinely mechanical/generated content into paths already excluded by the check (note: only subprojects/* is excluded, and the check only counts --diff-filter=M modified files — added/deleted files are not counted). Do not "retry" the job; it will deterministically fail until the diff shrinks or the policy changes.

Related: none

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 9d88efb8-edcf-4fa4-b5c2-c73f51e38844 in the triage console for the audit trail.

Signed-off-by: Lukas Humbel <lhumbel@nvidia.com>
@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentPR Size Check · commit 8effce4f

TL;DR: The "PR Size Check" workflow failed because PR #2200 modifies 897 lines in existing files (excluding subprojects), exceeding the repo's 500-line limit; fix by splitting the PR into smaller pieces or getting a maintainer waiver.

Full analysis

Summary: The check-pr-size job in .github/workflows/pr-size-check.yml exited 1 because the PR's modified-line count exceeds the enforced maximum.

Root cause: The workflow computes lines added to existing (modified) files via git show --numstat --pretty="" --diff-filter=M -- . ':(exclude)subprojects/*' and fails when the sum is greater than 500. For this PR the sum is 897, so the guard triggered exit 1 with "❌ PR size check failed! This PR adds 897 lines (excluding subprojects), which exceeds the maximum of 500 lines." This is a size-policy gate working as designed — not a build/test/infra failure. Note the count only covers --diff-filter=M (modified existing files), so new files don't count; the 897 lines are all in files that already existed.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #2200, branch uring-fixed-files) — the PR content itself, not a specific regression commit.

File: .github/workflows/pr-size-check.yml (the check-pr-size job's size-check step)

Suggested fix: This is not a code bug — the author needs to reduce the PR's footprint. Options:

  • Split PR feat(posix): Asynchronous Open in POSIX-uring #2200 into multiple smaller PRs each under the 500-modified-line limit.
  • Move large mechanical/generated changes into separate PRs, or into subprojects/* if appropriate (that path is excluded from the count).
  • If the change is genuinely atomic and can't be split, request a maintainer override/waiver for the size check, or adjust the 500-line threshold in pr-size-check.yml via a separate, approved change.

Related: none

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 9407e904-0b9a-4589-902d-954e97748bea in the triage console for the audit trail.

Signed-off-by: Lukas Humbel <lhumbel@nvidia.com>
Signed-off-by: Lukas Humbel <lhumbel@nvidia.com>
@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentPR Size Check · commit ccf96cf9

TL;DR: The "PR Size Check" gate failed because PR #2200 modifies 860 lines in existing files (excluding subprojects), exceeding the workflow's hard limit of 500. Split the PR into smaller pieces (or adjust/exempt the limit) to pass.

Full analysis

Summary: The check-pr-size job in .github/workflows/pr-size-check.yml exited with code 1 because the PR's line-change count exceeds the configured maximum.

Root cause: The workflow computes lines added to modified files with git show --numstat --pretty="" --diff-filter=M -- . ':(exclude)subprojects/*' and fails when the total is > 500. For this PR the total is 860 lines, so the guard triggered exit 1. This is a working-as-designed size gate, not a bug — the log shows the check ran instantly (13:46:41) with no hang and produced a deterministic verdict:

❌ PR size check failed!
This PR adds 860 lines (excluding subprojects), which exceeds the maximum of 500 lines.
##[error]Process completed with exit code 1.

Implicated commit: unknown (this is the PR's own content — the merge commit is 95958ba, merging [REDACTED:Hex High Entropy String] from branch uring-fixed-files; no single upstream commit is at fault).

File: .github/workflows/pr-size-check.yml (the check-pr-size step running the LINES_CHANGED > 500 check)

Suggested fix: This is a size-policy failure, not a defect. Resolve it by reducing the PR's footprint:

  • Split PR feat(posix): Asynchronous Open in POSIX-uring #2200 (uring-fixed-files) into smaller, logically-scoped PRs each under the 500-line limit.
  • If the change is genuinely atomic and cannot be split, ask a maintainer to raise the threshold or add an exemption/override label in pr-size-check.yml.
  • Note the check counts only added lines in modified files (--diff-filter=M); moving bulk additions into new files, or under subprojects/, would not count — but only do this if it reflects the real structure, not to game the gate.

Related: none

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id d5b04ec2-37d4-4eda-93a2-d924fa788ca8 in the triage console for the audit trail.

@svc-nixl

svc-nixl commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentAWS NIXL Validation · commit ccf96cf9

TL;DR: The AWS NIXL Validation job failed while building the aws-sdk-cpp dependency because a git submodule clone of https://github.com/awslabs/s2n.git failed ("could not read Username for 'https://github.com'"); that repo has been moved/renamed, so the anonymous clone of the stale submodule URL errors out. Pin/patch the s2n submodule to the current aws/s2n-tls URL (or use a newer aws-sdk-cpp tag whose .gitmodules points there).

Full analysis

Summary: Dependency build step failed cloning the crt/s2n submodule of aws-sdk-cpp (tag 1.11.760), aborting the whole NIXL test job.

Root cause: During git clone --recurse-submodules ... aws-sdk-cpp --branch 1.11.760, 13 of 14 CRT submodules cloned fine, but crt/aws-crt-cpp/crt/s2n from https://github.com/awslabs/s2n.git failed twice with fatal: could not read Username for 'https://github.com': No such device or address / fatal: clone of 'https://github.com/awslabs/s2n.git' ... failed. The awslabs/s2n repo was renamed to aws/s2n-tls; the old URL no longer serves anonymous clones and git falls back to prompting for credentials, which in non-interactive CI produces the "could not read Username" error. This is a dependency/environment failure, reproducible on any PR — not a hang (progress was continuous) and not caused by the code in PR #2200. After the AWS Batch job reported FAILED, the workflow exited 1.

Implicated commit: unknown (not a repo code change; the trigger is the external awslabs/s2n repo move interacting with the pinned aws-sdk-cpp@1.11.760 .gitmodules)

File: the CI script that runs git clone --recurse-submodules --depth 1 --shallow-submodules https://github.com/aws/aws-sdk-cpp.git --branch 1.11.760 (in the AWS NIXL Validation workflow / its build-dependencies script; exact path not in the fetched log)

Suggested fix: Make the s2n submodule resolve to its new home instead of the dead awslabs/s2n URL. Concretely, one of:

  • Add a global git URL rewrite before the recursive clone: git config --global url."https://github.com/aws/s2n-tls.git".insteadOf "https://github.com/awslabs/s2n.git", or
  • Clone aws-sdk-cpp without --recurse-submodules, run git submodule sync, patch the s2n entry in .gitmodules, then git submodule update --init --recursive, or
  • Bump the pinned aws-sdk-cpp tag to a newer release whose .gitmodules already points at aws/s2n-tls.

Related: none (no matching issue/PR found in this repo).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants