[TENT] Honor forced TCP in classic compatibility layer - #4027
[TENT] Honor forced TCP in classic compatibility layer#4027ToLiveAndLove wants to merge 1 commit into
Conversation
| TEST(TransferEngineTentCompatibilityTest, TcpProtocolForcesTcpTransport) { | ||
| ScopedEnvVar use_tent("MC_USE_TENT", "1"); | ||
| ScopedEnvVar force_tcp("MC_FORCE_TCP", nullptr); | ||
| ScopedEnvVar hostname("MOONCAKE_LOCAL_HOSTNAME", "127.0.0.1"); | ||
| ScopedEnvVar conf( | ||
| "MC_TENT_CONF", | ||
| R"({"transports":{"tcp":{"enable":false},"rdma":{"enable":false},"shm":{"enable":false},"hp_tcp":{"enable":false},"mpcomm":{"enable":false},"io_uring":{"enable":false}},"metrics":{"enabled":false}})"); | ||
|
|
||
| TransferEngine engine(true); | ||
| ASSERT_TRUE(engine.isUsingTent()); | ||
| ASSERT_EQ(engine.init(P2PHANDSHAKE, "compat-protocol-tcp", "", 0, "tcp"), | ||
| 0); | ||
|
|
||
| std::array<char, 4096> buffer{}; | ||
| ASSERT_EQ(engine.registerLocalMemory(buffer.data(), buffer.size()), 0); | ||
| EXPECT_EQ(engine.unregisterLocalMemory(buffer.data()), 0); | ||
| } |
There was a problem hiding this comment.
TcpProtocolForcesTcpTransport currently only checks that init(..., "tcp") returns 0 and that register/unregister succeed. That would still pass if protocol were ignored again, as long as the engine comes up with some transport.
This test is the only coverage of the classic shim path (TransferEngine::init + MC_USE_TENT=1). Could we make the assertion match the bug?
- Also cover
MC_FORCE_TCP=1withprotocol=""(that shim path has no unit test today;LegacyForceTcpEnvOverridesTentConfigonly hitsConfigHelper::loadFromEnv). - After
init, assert the TCP constraint actually stuck — e.g.transports/rdma/enable == false/transports/force_tcp == true, or that RDMA was not installed. Register/unregister alone does not prove we stopped selecting RDMA.
Without that, a regression that drops forceTcp() in the 5-arg init overload would still look green.
|
The only failing check, |
Description
Honor the classic Transfer Engine TCP constraints when existing callers run on
TENT through
MC_USE_TENT=1.Classic callers use either
MC_FORCE_TCP=1orinitialize(..., protocol="tcp")as a hard transport constraint. Both signalswere previously dropped by the compatibility path, allowing TENT to initialize
and select RDMA.
This change:
MC_FORCE_TCPinto TENT configuration;protocolargument through an ABI-safeinitoverload;policy lists RDMA first or lists only RDMA;
Both the configuration and selector changes are necessary. Configuration alone
cannot override a conflicting policy. Selector enforcement alone would still
initialize RDMA devices and consume their resources.
Refs #3995 (
P0-force-tcp)Negative validation
On unmodified
origin/main, with RDMA-capable hosts:MC_FORCE_TCP=1still initialized RDMA and attempted an mlx5 endpoint beforefalling back to TCP.
initialize(..., protocol="tcp")with an RDMA-only TENT policy loadedIbvLoaderand initializedmlx5_0andmlx5_2; TCP was not installed.Positive validation
With this change:
MC_FORCE_TCP=1installs TCP without creating an RDMA context.protocol="tcp"has the same behavior without requiringMC_FORCE_TCP.constraint.
Module
mooncake-transfer-engine)mooncake-store)mooncake-reshard)mooncake-ep)mooncake-pg)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-common)mooncake-rl)Type of Change
How Has This Been Tested?
Test commands:
cmake --build build \ --target tebench engine transport_uint_test \ transfer_engine_config_override_test tent_transport_selector_test \ config_lifecycle_test tent_transport_hint_test \ tent_tcp_datapath_roundtrip_test -j32 ctest --test-dir build \ -R 'transport_uint_test|config_lifecycle_test|transfer_engine_config_override_test|tent_transport_selector_test|tent_transport_hint_test|tent_tcp_datapath_roundtrip_test' \ --output-on-failureTest results:
Manual validation used two RDMA-capable hosts:
MC_FORCE_TCP=1: repeated WRITE+READ operations passed data verification.protocol="tcp"withoutMC_FORCE_TCP, with an RDMA-only configured policy:repeated 4 MiB WRITE+READ operations passed data verification.
ssconfirmed established TCP connections on both peers.fallback.
A separate
USE_TENT=OFFRelease build oftransfer_enginealso passes.Checklist
./scripts/code_format.shAI Assistance Disclosure
AI assistance was used. The author reviewed the code and ran the reported
tests.