Ub transport test for ci #3
Open
zchuango wants to merge 25 commits into
Open
Conversation
zchuango
force-pushed
the
ub_transport_test
branch
3 times, most recently
from
April 10, 2026 03:17
5f13769 to
841d92c
Compare
…transport_store_dev
…transport_store_dev
…locator.cpp Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
zchuango
force-pushed
the
ub_transport_test
branch
from
May 26, 2026 06:14
841d92c to
70cf348
Compare
zchuango
force-pushed
the
ub_transport_test
branch
from
May 26, 2026 09:29
5c32729 to
7a198b8
Compare
# Conflicts: # mooncake-store/src/utils.cpp
zchuango
force-pushed
the
ub_transport_test
branch
from
May 30, 2026 02:07
cf369d2 to
154df78
Compare
liangxu2000
pushed a commit
to liangxu2000/Mooncake
that referenced
this pull request
Jul 24, 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 zchuango#1 high, zchuango#2 medium). Item zchuango#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
1 participant
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
Module
mooncake-transfer-engine)mooncake-store)mooncake-ep)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-pg)mooncake-rl)Type of Change
How Has This Been Tested?
Checklist
./scripts/code_format.shbefore submitting.