CI: avoid /build comment to trigger blossom-ci - #748
Merged
Merged
Conversation
|
👋 Hi dpressle! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
Contributor
Author
|
/build |
dpressle
force-pushed
the
blossom_ci_comment
branch
from
September 2, 2025 06:13
5df8624 to
b4ebca6
Compare
We want to avoid having to add /build commnet to trigger blossom CI. This will tru only for branches: - release/* - main Signed-off-by: Daniel Pressler <danielpr@nvidia.com>
dpressle
force-pushed
the
blossom_ci_comment
branch
from
September 2, 2025 07:07
b4ebca6 to
8dae971
Compare
yosefe
approved these changes
Sep 2, 2025
Contributor
Author
|
/build |
svc-nixl
referenced
this pull request
Sep 7, 2026
Drive the process-wide allocator through device allocation, host-to-device and device-to-host copies, memset, explicit ownership release, and mapped host memory access. Skip cleanly when no CUDA-capable GPU is available so CPU-only test hosts can still run the unit suite. Signed-off-by: Tomer Davidor <tdavidor@nvidia.com>
This was referenced Sep 8, 2026
svc-nixl
referenced
this pull request
Sep 14, 2026
nixlBackendInitParams::customParams defaults to nullptr, and the base constructor dereferences it unconditionally. nixlAgent always fills the field, so this never fires in normal use; it fires immediately for a directly constructed engine, which is what a backend's own unit tests do. The crash lands in the base member-initializer list, before any derived constructor body runs, so a backend cannot guard against it on its own. The caller's fix -- pointing the field at an empty map -- is easy once you know it, and not discoverable from a declaration whose default is nullptr. This only stops the base class from crashing. It does not make a null customParams generally supported: gpunetio, obj and the Dell s3_accel engine dereference the pointer directly in their own constructors, and they require parameters, so what an absent-parameters backend should do there is a separate question. Left alone deliberately. Fixes #2245 Signed-off-by: Kyeongpyo Kim <hgichon@gmail.com>
svc-nixl
referenced
this pull request
Sep 14, 2026
nixlBackendInitParams::customParams defaults to nullptr, and the base constructor dereferences it unconditionally. nixlAgent always fills the field, so this never fires in normal use; it fires immediately for a directly constructed engine, which is what a backend's own unit tests do. The crash lands in the base member-initializer list, before any derived constructor body runs, so a backend cannot guard against it on its own. The caller's fix -- pointing the field at an empty map -- is easy once you know it, and not discoverable from a declaration whose default is nullptr. This only stops the base class from crashing. It does not make a null customParams generally supported: gpunetio, obj and the Dell s3_accel engine dereference the pointer directly in their own constructors, and they require parameters, so what an absent-parameters backend should do there is a separate question. Left alone deliberately. Fixes #2245 Signed-off-by: Kyeongpyo Kim <kpkim@gluesys.com>
svc-nixl
referenced
this pull request
Sep 14, 2026
Remove the default rc_gda exclusion and the argument scan that supported it. Keep --disable-ll-nvlink on the conditional RDMA invocation while allowing UCX to select the transport for default runs.
svc-nixl
referenced
this pull request
Sep 14, 2026
registerMemory() registered the destination MR with only FI_REMOTE_WRITE | FI_REMOTE_READ for all non-tcp/sockets providers. CXI only generates a target-side remote-write completion event when the destination MR key has events enabled (FI_RMA_EVENT). NIXL's libfabric backend protocol depends on that target-side completion (in combination with the SEND/RECV notification) to know a transfer has completed, so CXI destination MRs must be registered with FI_RMA_EVENT. Fix by adding a cxi-specific branch in registerMemory() that includes FI_RMA_EVENT in provider_access_flags. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved memory registration for CXI-based networking. * Preserved remote read and write access while enabling required remote memory events. * Other supported networking providers retain their existing memory access behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Ryan Hankins <ryan.hankins@hpe.com> Co-authored-by: Adit Ranadive <aranadive@nvidia.com>
svc-nixl
referenced
this pull request
Sep 15, 2026
## What? `MpStoreTest.ForeignOwnedStoreIsIgnoredAndNotReapable` and `MpExporterTest.ForeignOwnedLockFileCannotSilenceTheRun` `chown()` a file to uid 65534 and then assert `prometheus_mp` ignores it, treating a `chown` that returns 0 as proof the owner changed. On the v1.5.0 RC0 environments it is not: the call succeeds and the file keeps its owner, so the product sees `st_uid == geteuid()`, takes the healthy path, and both tests fail on an environment property. They now read the owner back and skip when it did not change, reporting the effective uid, the post-`chown` `st_uid`, the filesystem type, the seccomp mode and the uid map. Test-only; where the handover works the tests run and assert exactly as before. ## Why? NVBug 6734875 / NIX-1806: both fail on all four executed RC0 environments, x86_64 and aarch64. The `st_uid` checks they cover are correct and unchanged -- the tests were asserting a precondition they never verified. Mutation-tested: a no-op `chown` makes both skip with the diagnostic, and bypassing the read-back reproduces the reported failures. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved diagnostics for multiprocess telemetry test environments, including filesystem, security, and ownership details. * Added shared handling for transferring file ownership during test setup. * Tests now provide clearer reasons when required permissions or ownership changes are unavailable and skip appropriately. * Consolidated ownership management across related telemetry tests, reducing duplicated setup logic and improving consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
This was referenced Sep 15, 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.
We want to avoid having to add /build comment to trigger blossom CI. This will true only for branches:
What?
Update blossom-ci workflow to avoid having to add comment in order to trigger it
Why?
Improve usability for blossom ci
How?
New feature was enabled in Blossom ci