ci: stop non-etcd sanitizer stages from spawning the etcd/gRPC comm thread - #1831
Conversation
|
👋 Hi NirWolfer! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
…hread nixl_posix_test (and the other single-process install-dir binaries) failed flakily under ASan/UBSan, aborting inside libsanitizer with a thread-registry CHECK failure (sanitizer_thread_registry.cpp). Root cause: start_etcd_server exports NIXL_ETCD_ENDPOINTS for the whole script, and detectEtcd() makes every nixlAgent start an etcd comm thread when that var is set. That thread opens a gRPC channel whose WorkStealingThreadPool calls pthread_create, which flakily trips the GCC 13.3 libsanitizer thread-registry bug -- the same toolchain issue nixl_gusli_test and MetadataExchange are already excluded for. A POSIX file-backend test has no need for metadata exchange, so the etcd connection was purely incidental. Fix: run the only etcd-dependent install-dir binary (nixl_etcd_example) first, while the endpoints are still set, then unset NIXL_ETCD_ENDPOINTS (and the related vars) before the remaining single-process binaries. With no endpoints set, those agents never start the comm thread, so they no longer reach the libsanitizer pthread_create path -- making the stages deterministic while keeping full coverage (nothing is excluded). Signed-off-by: NirWolfer <nwolfer@nvidia.com>
1576717 to
af895f8
Compare
|
/build |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIn ChangesSanitizer Script Fix
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Summary
nixl_posix_test(and other single-process install-dir binaries) failed flakily under the ASan/UBSan sanitizer pipeline, aborting with no useful output on the teed logs. This fixes it without dropping any coverage by stopping those binaries from opening an unnecessary etcd/gRPC connection under the sanitizers.Root cause
The failure is the GCC 13.3 libsanitizer thread-registry bug — the same toolchain issue
nixl_gusli_testandMetadataExchangeTestFixtureare already excluded for — but the trigger was incidental:start_etcd_server(run for the gtest metadata-exchange tests)exportsNIXL_ETCD_ENDPOINTSfor the rest of the script.detectEtcd()(nixl_agent.cpp) makes everynixlAgentstart an etcd comm thread whenever that var is set.WorkStealingThreadPoolcallspthread_create, which flakily trips the libsanitizer CHECK.A POSIX local file-backend test has no need for metadata exchange — the etcd connection was purely a side effect of the leaked env var.
Captured backtrace (via a temporary diagnostic that wrote sanitizer reports to files):
Across a single 20-iteration diagnostic run the per-iteration exit codes were ~45% crashes (
rc=134SIGABRT /rc=139SIGSEGV), the rest clean — consistent with a flaky toolchain race, not a NIXL bug.Fix
nixl_etcd_example) first, while the endpoints are still set.unset NIXL_ETCD_ENDPOINTS NIXL_ETCD_NAMESPACE NIXL_ETCD_PEER_URLSbefore the remaining single-process binaries.With no endpoints set, those agents never start the comm thread, so they never reach the libsanitizer
pthread_createpath — making the stages deterministic while keeping full coverage (nothing is excluded;nixl_posix_teststill runs its complete-n 128 -s 1048576suite).Notes
nixl_etcd_examplegenuinely needs gRPC threads, so it remains the one stage still exposed to the same toolchain bug (it's the standalone equivalent of the already-excludedMetadataExchange). It's isolated up front now; if it proves flaky it can be excluded the same way.Summary by CodeRabbit