Conversation
Updated the UB Phase 3 Test Guide to clarify testing procedures, scope, and configuration for the UB_TENT branch. Enhanced sections on unit testing, integration testing, and common pitfalls.
Le1zyCatt
changed the title
github-actions
Bot
added
documentation
Le1zyCatt
pushed a commit
that referenced
this pull request
Jul 27, 2026
kvcache-ai#3029) * [TENT] Add transport labels to metrics for per-transport observability Switch all 16 TENT metrics from static to dynamic yalantinglibs types to add a `transport` label (and `from`/`to` on the failover counter), so operators can slice read/write bytes, requests, failures, latency, and failover paths by transport without grepping logs. Cardinality control: label values come exclusively from the TransportType enum closed set (11 values) via a pre-built lookup table indexed by enum. No arbitrary strings are accepted — the closed set is enforced at the type level. Total series upper bound ~1500, fully bounded. Changes: - 7 counters + 8 histograms → basic_dynamic_counter/histogram<int64_t, 1> with label "transport"; failover counter → <int64_t, 2> with "from"/"to" - counters_ vector widened to metric_t* base pointers (N varies 1 vs 2) - All record methods take TransportType; 10 call sites in transfer_engine_impl.cpp migrated (9 in recordTaskCompletionMetrics + 1 failover) - Macros and ScopedLatencyRecorder updated with transport parameter - getSummaryString()/getJsonMetrics() sum across labels for flat totals; per-transport breakdown available only via Prometheus /metrics - Label values aligned with TransportSelector::transportTypeName() Workaround: ylt basic_dynamic_histogram::serialize() calls str.clear() when all label combos have sum=0, wiping previously serialized content. Each metric is serialized into a temp buffer then appended to avoid this. ylt upstream is not modified. Also fixes pre-existing bug: shutdown() did not reset bound_http_port_, so httpPort() returned a stale port after a shutdown+reinitialize cycle. Verified: full build (USE_TENT=ON, TENT_METRICS_ENABLED=ON) clean; metrics_config_loader_test 25/25, tent_metrics_http_server_test 1/1, tent_metrics_recording_test 10/10; label landing confirmed via real TransferEngine path with {transport="tcp"} in Prometheus output. Signed-off-by: staryxchen <staryxchen@tencent.com> * docs(tent/metrics): document transport labels and per-transport queries Add Labels column to the Available Metrics table, a new Labels subsection explaining the TransportType closed-set scheme (11 values, from/to on failover, cardinality bounds), and update the Prometheus example output and Grafana queries to show per-transport filtering. Signed-off-by: staryxchen <staryxchen@tencent.com> * Address review: fix initialize() regression + add bounds-checked label lookup 1. initialize() regression fix: the rebase inadvertently reverted task#1's fix (commit 7dc87d8) that moved validateConfig before compare_exchange. The old order (CAS first, validate second, reset on failure) had a race window where concurrent callers see initialized_=true and return OK against an unconfigured singleton. Restored the correct order: validate first, CAS second, no reset needed. 2. Bounds-checked label lookup: added transportLabel(TransportType) helper that returns "unknown" if tp is out of range, replacing 9 direct kTransportLabelNames[tp] accesses. Defensive guard against memory corruption or a new transport type added without updating the table. Addresses gemini-code-assist review feedback on PR kvcache-ai#3029 (items #1 high, #2 medium). Item #3 (null check in sumCounterValues) intentionally not addressed — the pointers are class members, not dynamically allocated. Signed-off-by: staryxchen <staryxchen@tencent.com> * Address review: fix docs code examples + add per-transport label test M1: Update "Recording Transfer Metrics" and "RAII Latency Measurement" code examples in metrics.md to use the new API signatures with TransportType parameter. The old examples would not compile. M2: Add PrometheusLabelsDistinguishTransports test that records with RDMA and TCP transports, scrapes /metrics, and asserts per-transport labels appear as separate lines plus from/to labels on the failover counter. Addresses general-purpose-6 review feedback on PR kvcache-ai#3029. Signed-off-by: staryxchen <staryxchen@tencent.com> * trigger CI with run-ci label * Fix CI: guard kTransportLabelNames definition with TENT_METRICS_ENABLED The definition was outside the #if TENT_METRICS_ENABLED block, so building with TENT_METRICS_ENABLED=0 (the CI default) failed because the declaration (inside #if in the header) was not visible. Moved the definition inside the #if block. Signed-off-by: staryxchen <staryxchen@tencent.com> --------- Signed-off-by: staryxchen <staryxchen@tencent.com>
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.
Description
This PR continues the phased UB transport work for Mooncake Transfer Engine on the Kunpeng SuperNode.
Phase 1 introduced the initial
UbTransportand URMA endpoint implementation for the legacy Transfer Engine. Phase 2 added mock URMA support, unit tests, and CI/testing infrastructure for UB transport. This Phase 3 PR enables UB transport in TENT by adding a TENT-side UB transport adapter and the required control-plane integration.The main goal is to make UB available as a selectable TENT transport while reusing the existing legacy
UbTransportdata path. Instead of rewriting the URMA data path, this PR adds an adapter layer that maps TENT transport APIs, segment metadata, memory registration, transfer batches, and status queries to the existing UB transport implementation.Related work:
Major Changes
1. Add UB as a TENT transport type
This PR adds UB support to the TENT transport type system, including:
TransportType::UB"ub"string parsing in the TENT selectorThis allows TENT to recognize UB as a selectable transport type.
2. Add
UbTentTransportThis PR introduces
UbTentTransport, a TENT transport adapter for the existing legacyUbTransport.UbTentTransportimplements the TENT transport interface and delegates UB data path operations to the legacyUbTransport.It handles:
This keeps the existing URMA/UB data path mostly unchanged and limits this PR to the TENT integration layer.
3. Add
UbTentMetadataBridgeLegacy
UbTransportdepends on the oldTransferMetadatainterface, while TENT usesSegmentManagerand TENT-style segment descriptors.This PR adds
UbTentMetadataBridgeto bridge the two metadata systems. The bridge is responsible for:UbTransportThis allows the legacy UB transport logic to work under the TENT segment management model.
4. Add UB bootstrap support to the TENT control plane
UB requires endpoint bootstrap information exchange before data transfer.
This PR adds a UB bootstrap path to the TENT control plane, including:
RpcFuncID::BootstrapUbControlClient::bootstrapUb()ControlService::onBootstrapUb()With this change, UB endpoint bootstrap can be performed through the TENT control-plane RPC path.
5. Improve URMA memory registration and cleanup behavior
During UB/TENT integration, several URMA resource lifetime issues were found and fixed.
This PR improves:
urma_uninit()usage during per-context teardownThese changes make UB transport more stable in real URMA environments.
6. Add tests and documentation
This PR adds TENT UB related tests and documentation, covering:
The dual-node test path is documented in:
Scope
This PR focuses on enabling UB transport inside TENT by adapting the existing UB implementation.
This PR does not redesign the full UB data path and does not introduce a unified vendor device abstraction for topology discovery. Those can be handled as follow-up refactors.
Module
mooncake-transfer-engine)mooncake-store)mooncake-ep)mooncake-pg)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-common)mooncake-rl)Type of Change
How Has This Been Tested?
Build with UB and TENT enabled:
Test commands:
Run TENT UB unit tests:
Run UB transport tests with mock URMA or an available URMA environment:
For real Kunpeng UB hardware, the dual-node test path is documented in:
Test results:
Manual testing includes build verification and TENT UB unit-test coverage. Real dual-node UB/TENT validation should follow
ub_phase3_test_guide.md.Checklist
./scripts/code_format.shpre-commit run --all-filesand all hooks passAI Assistance Disclosure
AI tools were used to help draft and polish the PR description. The implementation, testing, and final verification are the responsibility of the human submitter.