Repository navigation
[Feature] Remove the legacy Monitor API and non-Prometheus metrics implementation #6923
Description
Activity
@warku123 The existing
/monitor/getnodeinfointerface already provides the node'sversionandchain_idinformation. If this endpoint remains available after the legacy metrics are removed, these values will not be lost as a result of removing the legacy metrics.In that case, why do we still need to expose the same information through the new
tron:node_info{version, chain_id}Prometheus metric? Is there a specific use case that requires these fields to be available as Prometheus metrics?@xxo1shine Thanks for the question — you're right that the values are not lost, and
/monitor/getnodeinforemains available. Thetron:node_infometric is not meant for API consumers; it targets the monitoring path, where the two serve fundamentally different models:- The HTTP API answers interactive, point-in-time queries for humans.
- Prometheus/Grafana alerting can only evaluate time series collected by scrape. Without a metric, node identity simply cannot participate in PromQL alert rules.
With the metric in place, common operational checks become plain alert rules:
# alert when a node is NOT on the expected chain (chain IDs differ per network, # e.g. mainnet vs Nile vs private — each has its own genesis block hash) absent(tron:node_info_info{chain_id="<expected genesis hash>"}) # overview of which networks the fleet is actually on count by (chain_id) (tron:node_info_info) # alert when a node still runs an old version after an upgrade window tron:node_info_info{version!="<expected release>"} == 1There is also a consistency argument: without the metric, every operator would have to build a custom poller/exporter that converts
/monitor/getnodeinforesponses into time series before Grafana could alert on them — which is exactly the kind of duplicated glue code this cleanup track is trying to remove. Exposing static info as an Info metric is a standard pattern (and java-tron already does it for JVM/OS info via the built-in Prometheus collectors).For what it's worth, the two fields were requested by node operators for our Grafana dashboards: show the running node version, and use
chain_idto quickly tell whether a node is connected to mainnet or a testnet. If one only wants to display the version, a Grafana JSON API datasource querying the HTTP endpoint would technically work, but that is not the standard alerting path and requires per-panel setup.So the metric complements the existing API rather than duplicating it: the API serves interactive queries, the metric serves automated monitoring and alerting.
Thanks for the detailed write-up. I support consolidating on Prometheus, but I think two blockers and a few migration details should be settled before implementation.
1. Stage the public API removal
node.metricsEnablehas been documented as deprecated since v4.8.2, but that does not explicitly deprecate theMonitorgRPC service or/monitor/getstatsinfo.Monitorstill returns data when the flag is enabled, so unregistering it is an observable breaking change.To follow principle 2 of #6921, I suggest:
- next release: mark
service Monitorandmessage MetricsInfodeprecated, emit a startup deprecation warning pointing tonode.metrics.prometheus.enable, publish migration guidance, and keep both APIs functional; - a future major release: remove the endpoints, proto definitions, legacy implementation, and Dropwizard dependency.
This differs from #6931:
WalletExtensionalready returnsUNIMPLEMENTED, whereasMonitoris functional when enabled.2. Keep block-fetch latency as the peer-selection signal
The per-IP histogram is updated independently of
node.metricsEnableinBlockMsgHandlerand consumed only byFetchBlockService; it is functional scheduling state, not part ofgetstatsinfo.Please move that state instead of replacing it with libp2p
avgLatency. The latter is a connection-lifetime mean of keepalive RTT, sampled only after 20 seconds without outbound traffic, and remains0before the first pong.shouldFetchBlock, however, interprets its estimate as block-fetch latency and subtracts elapsed fetch time from it. Substituting RTT therefore changes the signal, sampling, and zero-value behavior; it is neither equivalent nor demonstrably conservative.A bounded block-fetch estimator on
PeerConnection—for example, a small rolling reservoir with a cached P75—could be updated beside the Prometheus observation. Please cover unsampled peers, reconnect/reset, timeout fallback, and duplicate-request prevention in tests.3. Metric and migration details
- simpleclient
Infoappends_info; usetron:nodeas the collector base name (as in the draft) and querytron:node_info. - The proposed full value is a genesis block ID, whereas
eth_chainIdreturns only its last four bytes. Prefergenesis_block_idoverchain_id. - Include a legacy-field-to-Prometheus mapping, including fields without an equivalent, in the PR description and release notes, and land the documentation-en/zh updates before release.
For reviewability, I suggest separate PRs for (1) block-fetch state migration and tests and (2) deprecation/migration plus
tron:node_info. The actual removal can follow in the later major release.- next release: mark
Thanks for the careful review — both blockers accepted. Responses below.
1. Staged decommission of the Monitor API — accepted.
Phase Behavior Next release node.metricsEnablekeeps working exactly as before: when enabled, the gRPCMonitorservice stays registered and/monitor/getstatsinfokeeps serving. Add a prominent startup deprecation warning, mark the config key and both endpoints deprecated in docs and release notes, and publish the legacy field mapping table.Following release Remove the legacy stack: the config key (ignored harmlessly by the typesafe config afterwards), the gRPC service ( UNIMPLEMENTED), the HTTP endpoint (404), and the Dropwizard dependency.This follows principle 2 of #6921 (public APIs deprecated for one release cycle before removal) while committing to an actual removal date instead of an indefinite "future major".
2. Fetch-block latency signal — accepted.
Agreed: RTT alone does not capture block-serving throughput, so peer selection should keep a measured fetch-duration signal. #6923 has been updated to propose a bounded, per-connection estimator:
- a single
volatile longperPeerConnection, updated on every fetched block with the measured request-to-block duration:(ewma * 9 + last) / 10, clamped tofetchBlockTimeout; - the first sample is seeded from the channel RTT (
Channel.getAvgLatency()) so a fresh connection never sits in an unknown/zero state; - the state lives and dies with the
PeerConnectionobject — bounded by the active peer set, no disconnect bookkeeping, nothing stored in the metrics registry; - the Prometheus histogram stays unlabeled and observational only (a per-peer label would reproduce the unbounded-cardinality problem in the TSDB).
One factual note for the record:
Channel.getAvgLatency()is seeded at handshake completion and updated on keepalive pongs (pings fire only after 20s without outbound traffic), so it is not stuck at zero — but we agree it measures proximity, not serving quality, which is exactly why the estimator above becomes the selection signal.Other accepted points
- Single PR: the bounded fetch-duration estimator + peer-selection migration land together with the legacy stack removal and
tron:node_info{version, genesis_block_id}— the estimator replaces the histogram's functional consumer, so it is intrinsic to this cleanup rather than separable. Commits stay logically separated for review. - Rename the label
chain_id→genesis_block_id. - Legacy field mapping table in the PR description and release notes; docs (en/zh) updated alongside.
- a single
@warku123 Thanks, this resolves most of my concerns, and thanks for correcting the handshake detail. I verified that
HandshakeServiceseedsavgLatencyat handshake completion, so I retract my earlier statement that it remains zero until the first pong.A few points still need alignment before implementation:
-
Make the two phases consistent. Tracking: code refactor and cleanup #6921 requires removal in a future major release. "Following release" is fine only if that release is the agreed major; otherwise the exception should first be agreed in Tracking: code refactor and cleanup #6921. The current [Feature] Remove the legacy Monitor API and non-Prometheus metrics implementation #6923 description still specifies direct removal, a single removal PR, retained proto definitions, and silent handling of the removed config key. Please update it to reflect:
- Phase 1 PR: migrate the estimator, remove the per-IP histogram read and write sites, add
tron:node_info, addoption deprecated = truetoMonitorandMetricsInfo, emit deprecation warnings, and publish the mapping/docs while keeping both APIs functional. - Phase 2 PR in the target major: remove the HTTP/gRPC implementations, Dropwizard stack, and proto definitions, following the WalletExtension staging documented in Tracking: code refactor and cleanup #6921.
Because
/monitor/getstatsinfois registered independently ofnode.metricsEnable, a warning based only on config-key presence may miss HTTP consumers. Please also emit a process-once WARN when either deprecated API is first called. The removal release should retain a tombstone warning whennode.metricsEnableis present, so operators who skip Phase 1 do not lose monitoring silently. - Phase 1 PR: migrate the estimator, remove the per-IP histogram read and write sites, add
-
Keep EWMA initialization explicit. Using the channel latency estimate as a cold-start fallback is reasonable, but it should not enter the EWMA as if it were an observed block fetch. Keep an explicit unsampled state, use channel latency only as the fallback, let the first real fetch replace it, and apply EWMA from subsequent samples.
With the current
shouldFetchBlocklogic, clamping estimates tofetchBlockTimeoutalso makes the> fetchBlockTimeoutbranch unreachable while allowing an estimate equal to the timeout through the<=candidate filter. Please either preserve an over-timeout state or update the comparisons explicitly, with boundary tests. -
Validate the observable behavior change. The path is not categorically dormant in steady state: normal head+1 block retrieval arms
FetchBlockService, whose 50 ms worker may evaluate a secondary fetch while tracking remains active. Please instrument the normalized secondary-fetch rate and duplicate-block response count without peer labels, then report a before/after result from a sufficiently long Nile soak or a controlled delayed-peer experiment. The validation environment can be selected based on the observed event volume, but the test should not be dismissed on the premise that the path is dormant.
Minor documentation cleanup: describe
versionas "not currently exposed to Prometheus" rather than "lost", callgenesis_block_idthe full genesis block ID, and update the earlier PromQL examples totron:node_info{genesis_block_id="..."}.With these updates, I'm +1 on proceeding with Phase 1.
-
Thanks — and thanks for verifying the handshake seeding detail on your side.
Agreed on the overall structure. Going point by point:
1. Two-phase consistency. We'll update this issue to the Phase 1 / Phase 2 narrative:
- Phase 1: migrate the fetch-latency estimator and remove the per-IP histogram read/write sites; add
tron:node_info{version, genesis_block_id}; mark theMonitorservice andMetricsInfomessage withoption deprecated = truein the protos; emit deprecation warnings — both for thenode.metricsEnableconfig key and as a process-once WARN on the first invocation of either deprecated API (agreed that the HTTP endpoint is not config-gated, so a key-based warning alone would miss HTTP consumers); publish the legacy field mapping table and docs (en/zh). Both APIs remain fully available. - Phase 2 (in the agreed major release): remove the HTTP/gRPC implementations, the Dropwizard stack, and the proto definitions, following the WalletExtension precedent from Tracking: code refactor and cleanup #6921; keep a tombstone warning when
node.metricsEnableis still present, so operators who skip Phase 1 don't silently lose monitoring.
The concrete target major release needs an internal alignment round on our side — we'll update this issue (and the #6921 cross-references) as soon as it's confirmed.
2. EWMA initialization. Agreed — channel RTT is a different quantity and should not enter the series as an observation. We'll make the unsampled state explicit: channel latency serves only as a read fallback while unsampled, the first real fetch replaces the placeholder, and EWMA starts from the second sample. (The current implementation blends the first sample at α = 0.1 as outlier defense; we'll switch it to replace-on-first-sample and adjust the pinned tests accordingly.)
For the steady-state EWMA weight we plan to keep α = 0.1 (each new sample contributes 10%): at the expected sampling cadence a large degradation flips the
min()ordering within one or two samples, and the clamp plus the hard-timeout branch bound the downside, so the conservative weight mainly buys smoothing against per-sample noise (fetched block sizes vary by orders of magnitude). Would you see a reason to weight recent samples more aggressively here?Separately — the clamp/reachability issue you flagged is already fixed in the current implementation:
shouldFetchBlocknow has an explicit hard-timeout branch (unconditional switch once the wall-clock budget is exhausted) plus a latency-saturation gate that requires a strictly better candidate, with boundary tests covering both. Your comment referenced the earlier issue text.3. Verification. Agreed that this cannot be skipped. We'll add the two unlabeled metrics (normalized secondary-fetch rate and duplicate-block response count) and report before/after from a controlled experiment with delayed peers; based on the observed event volume we'll assess whether a longer Nile soak adds signal beyond the controlled run.
Docs nits all accepted: describe
versionas "not currently exposed to Prometheus" rather than lost; describegenesis_block_idas the full genesis block ID; update the PromQL examples totron:node_info{genesis_block_id="..."}.We'll follow up with the issue description rewrite and a Phase 1 PR reflecting all of the above.
- Phase 1: migrate the fetch-latency estimator and remove the per-IP histogram read/write sites; add
@warku123 Thanks, we are aligned. Three short points, then I will leave the remaining implementation details to the PR.
Alpha. I would not increase
alphaa priori. With replace-on-first-sample, the first measurement enters unsmoothed, and at0.1it can dominate the estimate for several subsequent fetches, so one unusually large block may skew a peer's early ranking.If you want warm-up protection, use a running mean for the first ten measured samples, equivalent to
alpha_n = max(1.0 / n, 0.1), and then continue with the fixed EWMA. The sample counter can be capped at ten.Either approach is acceptable for Phase 1. Please make the weight a named constant, document the rationale, and pin both degradation and recovery convergence in tests.
Selection logic. Agreed that my earlier comment referred to the previous issue text. Until the PR is opened, the revised issue should remain the source of truth. Please describe the final candidate selection, saturation gate, hard-timeout branch, and tested boundary cases there.
One intentional behavior change should also be listed explicitly under principle 1 of #6921: previously, a candidate whose P75 exceeded
fetchBlockTimeoutwas filtered out. With the new clamped EWMA, a saturated candidate remains eligible and may receive the secondary request at the hard timeout when no better candidate exists. I think this is a reasonable liveness improvement, but it should be documented and tested rather than left implicit.Verification and timing. The controlled experiment works for me. Please let the estimator complete its warm-up, then degrade one peer during the run and report how many additional per-peer samples are required for the ordering to change. Use the same harness and topology for the
developbaseline. If the one-or-two-sample claim is retained, please state and demonstrate the latency ranges for which it holds. Exact primary/secondary attribution for the experiment can come from the harness itself.Expose the underlying events as unlabeled, monotonically increasing Prometheus counters: fetch trackings armed, secondary fetches sent, and duplicate or already-known block responses. Derive normalized rates in PromQL rather than precomputing them on the node.
Please define the duplicate-response event precisely in the PR. A matching
advInvRequest.remove(item)combined with the block already being present can provide a best-effort "requested response for an already-known block" signal, but it does not by itself prove that the response was caused by a secondary fetch and may miss concurrent arrivals. If exact attribution is required, the secondary origin should be recorded explicitly in bounded request metadata; otherwise the metric should be named and documented as a generic already-known response counter.Phase 1 should target the next release so that the API deprecation window starts independently of the Phase 2 schedule. Once the target major is agreed, please record it in #6921 and cross-reference it here.
Looking forward to the Phase 1 PR.
Thanks — confirmed on all points. The remaining details will be folded into the Phase 1 branch before the PR opens.
Counters. The third counter will be added:
tron:block_fetch_armed(incremented whenFetchBlockServicearms a fetch tracking), alongsidetron:block_fetch_secondaryandtron:block_already_known. Per your precision note, the duplicate counter will be renamed totron:block_already_knownand documented as a best-effort "response for an already-known block" signal — we do not record secondary origin in request metadata, so it covers concurrent arrivals and any low block, not just secondary-fetch responses.On lifecycle: these three counters are verification-scoped instrumentation — they ship with Phase 1 so the before/after comparison and the deprecation-window soak both have identical signals, and they are scheduled for removal in Phase 2 once the results are recorded in this issue. The permanent fetch-latency signal is the existing unlabeled
tron:block_fetch_latency_secondshistogram, which stays.Alpha. α stays fixed at 0.1, expressed as a named constant (no pre-tuning, no warm-up running mean, per your note that both options are acceptable; the simplicity trade-off is stated in the PR). The constant carries the design rationale in a comment, and degradation/recovery convergence tests pin both directions.
Selection logic. The issue will be rewritten as the source of truth before the PR opens: final candidate selection, saturation gate, hard-timeout branch, and the measured boundary cases. The deliberate behavior change (saturated candidates stay eligible instead of being filtered like
P75 > fetchBlockTimeoutwas) is listed explicitly under principle 1 of #6921, with tests.Verification. As specified: warm up the estimator first, then degrade a single peer mid-run (tc netem) and report how many additional per-peer samples it takes for the ranking to flip; the develop baseline runs the same harness and topology, with the counters cherry-picked onto an instrumentation-only branch so both arms expose identical metrics. We will either qualify the "1–2 samples to flip" claim to the latency range where it holds, or replace it with measured numbers. Verification results will be posted back to this issue.
Timing. Phase 1 targets the next release so the deprecation window starts independently of Phase 2; once the target major is agreed, we will record it in #6921 and cross-reference it here.
@warku123 Thanks. The fixed alpha and the controlled comparison against an instrumentation-only develop baseline work for me. Two clarifications on the counters before we move the remaining review to the PR:
-
Please keep these counters beyond Phase 2. They describe fetch behavior that remains after the legacy stack is removed, and operators need them to detect regressions after release. The existing latency histogram cannot recover the secondary-fetch ratio or the already-known response count. Removing them later should be a separate observability decision, rather than part of the Dropwizard cleanup.
-
Please narrow the definition of
tron:block_already_known. A block below the current head is not necessarily already known; it could be an unseen fork block. For the proposed response counter, count a matching outstanding adv request whose exact block ID is already known before processing that response. Document that concurrent arrivals may be missed and that this does not establish secondary-fetch attribution. Exact attribution can remain in the experiment harness.
With those clarifications reflected in the issue rewrite, I'm +1 on proceeding with the Phase 1 PR. Please post the verification results here before merge so they can inform the final review.
-
Verification results from the controlled experiment, as discussed.
Setup. 4-node private chain (1 witness producer, 1 observer, 2 candidate peers) in docker on a single host. The observer's requests to the producer were degraded with source-IP-filtered
tc netem(only the observer→producer direction was delayed; candidates were untouched). Both arms ran the same harness and topology: baseline =develop+ the three counters on an instrumentation-only branch; treatment = the Phase 1 branch. Each delay step ran ~45s (≈15 blocks at 3s), ascending order.How to read the table. Each cell reports how many of the ~15 fetches armed in that window ended in a secondary-fetch switch — the observer giving up on the (degraded) producer and re-requesting the same block from a healthy candidate.
0/15means every fetch in the window kept hitting the degraded producer;15/15means every armed fetch switched away.Secondary-fetch switches per window:
added delay (observer→producer) develop + P75 Phase 1 EWMA 0 (control) 0/15 0/15 10ms 0/15 0/15 50ms 8/15 0/15 100ms 0/15 † 1/15 150ms 0/15 † 15/15 200ms 0/15 † 15/15 300ms 0/15 † 15/15 500ms 15/15 † 15/15 1000ms 15/15 † 15/15 2000ms 15/15 † 15/15 † Baseline points at 100ms and above come from an earlier round with the same harness; the low-tier points (≤50ms) and the treatment arm's 1000/2000ms points come from a later round on an updated branch (the peer-selection logic is identical — only the
already_knownwrite point differs between the two treatment builds).Observations.
- In both arms, every switch targeted the same healthy candidate, and each block produced at most one secondary request — no thrashing between candidates.
- In the treatment arm, switches fired ~25–50ms after arming (as soon as a candidate advertisement was available), well before the hard timeout; the baseline switched at comparable points once its P75 crossed the fetch timeout.
- Healthy segments — including two producer pauses and one all-direction degraded window with no eligible candidates — produced zero secondary fetches on either arm; the no-candidate clearing path handled the latter without issuing requests.
- After the 2000ms window was released, ~44 subsequent healthy blocks produced zero switches: the estimator recovered with no residual switching.
- Chain production and sync were unaffected in every window on both arms.
The 50ms row is worth calling out. The baseline switched in 8/15 fetches at 50ms in this run, yet switched 0/15 at 100ms in the earlier round — same build, same harness. This is the empty-histogram defect in action: a candidate that has never served a fetch reads P75 = 0.0 (a registry lookup auto-creates an empty histogram), so an unsampled candidate is always ranked "fastest" and the comparison branch degenerates into a race between the 50ms worker tick and the producer's P75, decided by the composition of the exponentially-decaying reservoir (how many low samples are stacked below the new degraded ones). Whether a given run switches is history-dependent. The EWMA arm reads unsampled candidates with the channel-RTT fallback instead of 0, so its noise floor is clean: no switching at 50ms, and consistent switching once degradation actually pushes the estimate past the comparison band.
Calibration.
- This is a single ascending run per arm — we report it as an observed divergence band under this topology, not as fixed universal thresholds.
- The private chain uses
fetchBlockTimeout = 200; the mainnet default is 500, so absolute thresholds scale proportionally. - Same-host baselines are ~1ms with near-empty blocks, so bandwidth variance (a real factor with full-size blocks) is not exercised; degradation was constant delay rather than jitter or loss.
already_knownwas validated live at the high-delay steps (switches and late duplicate responses matched one-to-one under the exact-ID definition); low-delay windows produced no such responses, as expected.
In every tested regime the new estimator was at least as good as the old signal, and strictly better in the mid-degradation band; the structural wins (O(1) read, bounded per-connection memory, no metrics-gate bypass, no unbounded per-IP keys) hold regardless of topology.
Next: the Phase 1 PR will be opened against
developreferencing this issue, and its link and CI status will be synced here.Thanks for running this and updating the issue. The reported no-thrashing and no-candidate behavior are useful results. My support for opening the Phase 1 PR stands; three points need clarification before these results support the final merge decision.
-
Please reconcile the 300 ms baseline row. With
fetchBlockTimeout = 200, a tracking that remains pending should trigger a secondary request at a subsequent worker tick if an eligible candidate is available (theoldPeerSpendTime >= fetchTimeOutbranch inshouldFetchBlock). Eligibility requires more than an advertisement: the candidate must also be idle and pass the baseline P75 filter. Please state the effective timeout and exact commits for each round, and provide enough decision traces to explain the 0/15 result. Repeat the key comparisons under matching configuration and warm-up conditions, including representative cases at the default 500 ms timeout. -
Please report the cost and benefit together. Include armed, secondary, and already-known counts per window for both arms, alongside time from the initial request to the first usable block and the extra response traffic. More secondary requests may reduce latency while increasing duplicate transfers; the switch fractions alone do not establish "at least as good in every regime".
-
Please show the estimator's adaptation directly. The ascending windows share history, so a 15/15 window does not establish how many new samples caused the decision to change. Record the old and candidate estimates, elapsed fetch time, and decision reason during degradation and recovery, and include a case with a controlled, non-negligible candidate RTT so the fallback and the estimate actually compete beyond the same-host setup. Then update or remove the "1-2 samples" statement in the issue accordingly. Similarly, zero switches during the subsequent healthy blocks does not by itself establish that the estimate recovered; prompt arrivals may simply have cleared tracking before a decision was made. The fixed worker interval and other timing inputs also mean that neither proportional threshold scaling nor an unchanged mainnet threshold follows from this experiment.
Please link the harness, configurations, and raw results in the PR. These targeted checks should give us the evidence needed to assess the latency/overhead tradeoff before merge.
-
Reply v8 — Round-3 re-run: baseline vs phase1 switch behavior under added latency
1. Background: scope of this reply
This reply follows up on the three points from the last review comment (2026-09-16). As requested, the key comparisons were repeated under matching configuration and warm-up conditions, including the default 500 ms timeout: a full re-collection of both builds, with each build collected at two effective
fetchBlock.timeoutvalues, 200 ms and 500 ms.The repeated runs use the same per-window controls in both builds:
- jar identity is pinned and verified at launch, with a fetch-block-service / PeerConnection instrumentation check that differs per build;
- containers are recreated before the run and verified (mounts, jar match, and configuration read-back);
- each measurement tier takes counter snapshots strictly before and after the netem window, and the netem on/off operations are timestamped;
- warm-up is gated and primary-peer reachability is confirmed per tier;
- the timeout configuration is confirmed by in-container HOCON read-back: the t200 runs use
fetchBlock.timeout = 200and the t500 runs usefetchBlock.timeout = 500, so both timeout settings are stated directly; - each run writes to its own UTC-stamped run directory and the chain is fail-stop.
Coverage of the three points in this reply: point 1 is covered — the repeat under matching conditions including the 500 ms default, and the per-window decision mix (the histogram columns in §3 are the aggregated form of the decision traces). Point 2 is partly covered — the per-window armed and secondary counts are in the §3 table (
armed(Δ)andsecondaryΔ/putΔ). Point 3 — the per-window decision breakdown in section 3 is the direct answer here; nothing further is required to evaluate the replacement.- baseline: one run per timeout setting (t200, t500), both completing without error;
- phase1: one run per timeout setting (t200, t500), both completing without error.
2. Method
Each build runs two timeout settings: t200 =
fetchBlock.timeout200 ms, t500 = 500 ms (the default). 14 windows per build, 28 windows total (2 builds × 2 timeouts × 7 tiers). Each window is one tier delay applied to the node-1↔peers path for 45 s while the node fetches blocks; 15 fetch requests are armed inside the window (armed counter POST−PRE = 15 throughout, except two phase1 windows noted below).Reduction (script
reduce_r3.py):perTier.tsvis parsed by metric name (column order is not stable).- The trace window is the netem on/off interval from the metrics snapshot, mapped through the time of day onto the container clock (
HH:mm:ss.SSS, UTC), with ±1 s tolerance. A window that would cross UTC midnight is rejected outright. - Numerator:
- baseline build: the window's lines recording a switch to the secondary peer being applied, with the paired annotation asserted equal;
- phase1 build: the window's lines recording a switch decision with the send flag set.
- Denominator: the armed-counter POST−PRE delta.
- Cross-checks (hard BLOCKED, never smoothed): trace window count vs the secondary-counter POST−PRE delta; switch-count consistency; each build's own trace markers must not appear in the other build's traces; armed delta must be 15 for baseline.
Every tier in both baseline runs passed all cross-checks. One phase1 cell is BLOCKED and is reported as-is (section 3).
3. Results (28 windows)
The table has four series — the 4 build × timeout combinations, named in the
seriescolumn — and 7 rows per series, one per tier: the tier is the additional netem delay applied for that 45 s window. For each row,armed(Δ)is the window'stron:block_fetch_armed_totalPOST−PRE delta, i.e. the fetch requests armed inside the window;switchedis the numerator — the baseline side countsput=trueswitches to the secondary peer, the phase1 side countssent=trueswitch decisions; andratiois switched ÷ armed(Δ).The histogram columns count trace events inside the window by classification, not requests: the baseline side classifies them by the
branch=field, the phase1 side by thedecision=field;failoccurs only in baseline rows andcomparison-fail/saturation-gateonly in phase1 rows, while the remaining classification names are shared columns that each row fills from its own taxonomy and leaves 0 otherwise. Because they are event counts, they need not add up toarmed(Δ). The last column,secondaryΔ/putΔ, is the tsvtron:block_fetch_secondary_totalPOST−PRE delta and equals the numerator in every non-BLOCKED cell — the cross-check; the one BLOCKED row is explained below the table.series tier(ms) armed(Δ) switched ratio hard-timeout comparison-pass comparison-fail fail saturation-gate no-candidate secondaryΔ/putΔ base-t200 100 15 13 13/15 0 13 0 4 0 1 13 base-t200 150 15 12 12/15 0 12 0 9 0 2 12 base-t200 200 15 12 12/15 12 0 0 12 0 3 12 base-t200 300 15 15 15/15 15 0 0 11 0 2 15 base-t200 500 15 15 15/15 15 0 0 11 0 2 15 base-t200 1000 15 15 15/15 15 0 0 15 0 1 15 base-t200 2000 15 15 15/15 15 0 0 11 0 1 15 base-t500 100 15 12 12/15 0 12 0 6 0 3 12 base-t500 150 15 12 12/15 0 12 0 9 0 0 12 base-t500 200 15 12 12/15 0 12 0 11 0 3 12 base-t500 300 15 12 12/15 0 12 0 17 0 2 12 base-t500 500 15 12 12/15 12 0 0 29 0 0 12 base-t500 1000 15 15 15/15 15 0 0 30 0 2 15 base-t500 2000 15 15 15/15 15 0 0 28 0 3 15 phase1-t200 100 15 0 0/15 0 0 28 0 0 2 0 phase1-t200 150 16 11 11/16 0 11 11 0 0 3 11 phase1-t200 200 — — BLOCKED — — — — — — — phase1-t200 300 15 14 14/15 2 9 8 0 3 3 14 phase1-t200 500 15 15 15/15 1 4 4 0 10 4 15 phase1-t200 1000 15 15 15/15 1 2 4 0 12 2 15 phase1-t200 2000 15 14 14/15 1 0 4 0 13 2 14 phase1-t500 100 15 0 0/15 0 0 29 0 0 2 0 phase1-t500 150 15 4 4/15 0 4 32 0 0 2 4 phase1-t500 200 15 12 12/15 0 12 12 0 0 3 12 phase1-t500 300 15 12 12/15 0 12 12 0 0 2 12 phase1-t500 500 15 13 13/15 1 12 19 0 0 3 13 phase1-t500 1000 15 14 14/15 1 6 9 0 7 1 14 phase1-t500 2000 15 15 15/15 1 2 10 0 12 3 15 phase1-t200tier 200: the ±1 s window contains 8sent=truedecisions while the tsv counter delta is 7. The extra decision line sits 37 ms after theNETEM_OFF/ POST-counter snapshot timestamp (01:54:00.037vsTS_UNIX=1789696440), i.e. a decision logged just after the POST snapshot was taken; the corresponding counter increment is not in the POST value. Per the reduction rules this is a cross-check mismatch and the cell is emitted as BLOCKED with the raw counts; it is not smoothed into either number. The remaining 27 cells are consistent.Two phase1 windows (t150 and t200 of the same run) show armed delta 16 instead of 15 (boundary armed event recorded on the other side of a snapshot); these windows keep the actual delta as denominator and are flagged as warnings. All baseline windows are 15.
4. Findings
Stated strictly within what the 28 windows support:
- Baseline saturates from 300 ms in the t200 group and from 1000 ms in the t500 group, reaching 15/15 in those segments.
- Baseline records 12–15
hard-timeoutevents per window at 200–2000 ms (t200 group) and 500–2000 ms (t500 group); phase1 records at most 2 in the same ranges. - At 100 ms the baseline switches 13/15 and 12/15 while the phase1 build records 0/15, with comparison-fail dominated decisions.
- The two phase1 runs differ at the 150 ms tier, 11/16 vs 4/15.
5. Overall conclusion
The 28-window round-3 data supports replacing the original selection logic with the new computation. Three reasons:
- Switch coverage is not reduced. In every latency region where the baseline build saturates, the saturation segments from 300 ms upward in the t200 group and from 1000 ms upward in the t500 group, the phase1 build switches on 14–15 of the 15 armed requests, matching or coming close to the baseline's 15/15.
- The main failure mode is eliminated. In those same segments the baseline records 12–15 hard-timeout events per window, waiting out the timeout before switching; the phase1 build switches proactively on the saturation-gate decision, hard-timeout drops to at most 2 per window, and every saturation-gate switch has sent=true. Hard-timeout remains in place as a fallback path and stays active.
- Gate behavior is by design, not a defect. In the 100 ms tier the phase1 build records 0/15 switches and its decision records are dominated by comparison-fail, meaning no switch when the candidate estimate is not better; no-candidate events are all sent=false and consume no switch budget. Across all 28 windows both builds kept the chain moving with no stall, and no functional regression was observed. Which build fares better in this tier is an end-to-end question outside this experiment's scope.
Supplementary characterization is not a prerequisite for the replacement judgment. The single BLOCKED cell and the 150 ms run-to-run variance reported verbatim in section 3 are data-integrity notes and do not change the conclusion above.
One-line conclusion: within the tested envelope, the new computation preserves the switching capability of the original selection logic and removes its main failure mode; the current data supports the replacement.
Thanks for the full re-collection. The round-3 results address the earlier 300 ms baseline concern for the current comparison, and should supersede the mixed-round comparison. Explicitly reporting the BLOCKED cell is also helpful.
Two points remain before drawing the replacement conclusion:
-
The baseline label does not establish timeout waiting. In
instrumentation/trace-counters,hard-timeoutcovers botholdPeerTop75 > fetchTimeOutandoldPeerSpendTime >= fetchTimeOut, whereas Phase 1 separates the saturation gate from the elapsed timeout. Their counts therefore cannot directly demonstrate that timeout waiting was eliminated. Please confirm the run commits for both instrumentation branches. If they match the inspected instrumentation, the existingspendMs/spendTimeMsfields on successful-switch lines should allow an approximate arming-to-secondary-dispatch comparison for both arms without rerunning the experiment. -
Please explain the 100/150 ms decisions from the recorded state.
comparison-faildoes not necessarily mean the candidate estimate was worse; the comparison uses the estimated remaining time and the 0.5 factor.instrumentation/trace-phase1already recordsspendTimeMs,oldEstimateMsandcandidateEstimateMsper decision, andupdateFetchLatencylogs every estimator update. Please extract representative sequences, including window-entry state and whether the candidate was still on its channel-latency fallback. The same update lines should support or qualify the issue's "1-2 samples to clamp" claim; aggregate branch counts cannot establish it.
The outcome check from the previous review remains: for representative matched windows, including the default 500 ms timeout, please report already-known counts alongside armed/secondary counts, initial-request-to-first-usable-block time, and extra response traffic. Fewer secondary requests may trade latency for lower overhead, but the current table does not establish either outcome. There is no need to expand the full delay matrix.
Please narrow the performance conclusions to what those measurements support, and link the exact commits, harness,
reduce_r3.py, configurations, and raw results in the PR.I remain supportive of opening the Phase 1 PR. The remaining evidence can be reviewed there before the merge decision.
-
- linked a pull request that will close this issuerefactor(metrics): phase 1 deprecation of the legacy monitor stack #6988
on Sep 22, 2026 @waynercheung Both points answered from the same 28 round-3 windows (no rerun), plus the outcome check. Numbers below are extracted by the
analysis-r4scripts over the four canonical run directories; cross-checks passed 30/30 (analysis-r4/verify.md) and 28/28 (per-block reconciliation inoutcome-latency.md).1. Run identity
series jar sha256 instrumentation branch commit base-t200, base-t500 c6f7f91af8d2ef5905ee3ca3e76b0377c92791e9a4d8553829b9b2c5ddee68c8instrumentation/trace-countersacefb07179912d8d943bc9ed4ab76fcae96f20ffphase1-t200, phase1-t500 ac84cb6411c2e7af52f113c8c221ca9235e35784c93d5ba7d6404520dfdb0d8finstrumentation/trace-phase12a49bdb60808aef3ef1d044a14f4d61158c7eee6Jar SHA-256s are taken from each run's
JAR_IDENTITYlog line. Caveat stated plainly: the jar↔commit correspondence is by build convention —chain7-baseline.sh/chain7-phase1.shbuild the named jar from the checked-out branch and hand it to the node; the jars were not retained after the runs, so the mapping is a convention mapping, not a re-hash. Both instrumentation branches were verified clean and equal to their remote SHAs at analysis time.2. Point 1 — timeout waiting, read from the spend fields
You are right that the counts alone cannot establish it. The
spendMs(baseline) /spendTimeMs(phase1) fields on successful-switch lines resolve it, and they cut in both directions:- Baseline
hard-timeoutswitches do not predominantly wait out the timeout: medianspendMsis 21–48 ms across every tier — those are the earlyoldPeerTop75 > fetchTimeOuttriggers (the first of the two conditions you named), not the elapsed one. Elapsed-timeout waiting appears only in the tail: p90 ≈ 218–227 ms (t200 group, tiers ≥ 300) and ≈ 513–531 ms (t500 group, tiers 1000/2000), i.e. at the configured timeout. - Phase1
hard-timeoutswitches shrink to 8 events across both runs combined, and all 8 sit at full length (233–250 ms in t200; 500–513 ms in t500). The switch load moves tosaturation-gate(median 8–41 ms) andcomparison-pass(median 22–53 ms).
Pooled per-tier medians (both runs per arm):
tier (ms) base hard-timeout med p1 hard-timeout med p1 saturation-gate med 300 33.0 236.5 8.0 500 35.0 366.5 31.5 1000 46.0 377.0 38.0 2000 37.0 377.0 35.0 So the corrected statement is: median arming→secondary-dispatch is similar in both arms (tens of ms); what changes is the tail. Baseline pays ≈ full-timeout waits on its p90 tail at high tiers; phase1 reroutes those decisions through the saturation gate / estimator comparison (typically ≤ ~60 ms), leaving the hard timeout as a fallback that fires at most twice per window and then at full length.
3. Point 2 — the 100/150 ms decisions, from recorded state
Window-entry state (phase1-t200, tier 100, first decision lines of the window):
decision=comparison-fail spendTimeMs=16 oldEstimateMs=4.0 candidate=/172.22.0.13 candidateEstimateMs=52.0 sent=false decision=comparison-fail spendTimeMs=66 oldEstimateMs=4.0 candidate=/172.22.0.14 candidateEstimateMs=46.0 sent=false updateFetchLatency peer=/172.22.0.11 firstSample=false oldMs=4 sampleMs=105 newMs=14The old peer enters with a warmed EWMA (4 ms, from pre-window samples); the candidates are still on their fallback values (46–75 ms across these windows; the channel-RTT fallback per the design). One 105 ms sample moves the EWMA to 14 ms — α = 0.1 closes 10% of the gap per sample.
Representative convergence (phase1-t200, tier 150, 01:51:12–01:51:21): three
updateFetchLatencylines interleave withcomparison-faildecisions — 5.0 → (s=152) 19 → (s=153) 32 → (s=152) 44 — and the next fetch opens with:decision=comparison-pass spendTimeMs=13 oldEstimateMs=44.0 candidate=/172.22.0.14 candidateEstimateMs=8.0 sent=truephase1-t500 tier 150 behaves the same at a higher plateau: 86 → 92 → 98 → 103 across three samples, then
comparison-pass spendTimeMs=6 oldEstimateMs=103.0 candidateEstimateMs=41.0.The fail/pass pair with identical estimates is the direct answer to "comparison-fail does not necessarily mean the candidate estimate was worse":
01:51:18.169 decision=comparison-fail spendTimeMs=152 oldEstimateMs=44.0 candidateEstimateMs=8.0 sent=false 01:51:21.025 decision=comparison-pass spendTimeMs=13 oldEstimateMs=44.0 candidateEstimateMs=8.0 sent=trueSame estimates, different elapsed time — the outcome tracks the estimated-remaining-time comparison you described (with the 0.5 factor), not the raw estimate ordering.
Absence, attested rather than inferred:
saturation-gate/hard-timeoutdecisions withsent=trueoccur only at tier ≥ 300 (t200) / ≥ 500 (t500); tiers 100–150 record none, and tier 100 records zerocomparison-passin both runs. When the hard timeout does fire, the old estimate is far below the cap (t200 tier 300: spend 235 ms, oldEstimate 6; t500 tier 500: spend 500 ms, oldEstimate 54) — a wall-clock trigger, not estimator saturation. In the latter case the following update shows the α = 0.1 pace explicitly:oldMs=54 sampleMs=504 newMs=99.On the "1–2 samples to clamp" claim, the update lines split it:
- For an unsampled connection it holds by construction — the first measurement replaces the fallback with the clamped sample (
firstSample=true oldMs=0 sampleMs=3 newMs=3is the logged small-scale case; a first sample above the timeout clamps in one step). - For a warmed estimator it does not: at α = 0.1, reaching 0.8×tier from a low start needs ≈ 16 samples at the tier value, and the 45 s windows supply only 13–16 samples for the delayed peer (fewer for the candidates). In all 35 run×tier×peer cells the estimate never reaches 0.8×tier inside the window — 18 cells are structurally unreachable (0.8×tier above the run's clamp: t200 tiers ≥ 300, t500 tiers ≥ 1000), and 17 are sample-starved, sometimes by 1 ms (t200 tier100 old peer: 14,22,30,37,43,49,54,58,62,66,69,72,75,77,79 — stops at 79 against a threshold of 80).
The issue text will be corrected to that wording.
4. Outcome check — matched windows, default 500 ms timeout
Counters are window POST−PRE deltas. "First usable" is per-block first arrival, taken from the native
Receive block/interval … fetch/delaylines that exist in both builds (matched by message text, since the instrumentation shifts the line number 111↔119). Those lines are one per (block, arrival) — after a switch the same block often arrives twice (block 24556, phase1-t200 tier 500: 503 ms from the delayed peer, 2 ms from the switched peer) — so per-block first arrival is the time to the first usable copy.t500 run (the default-timeout arm), tiers 100/150/500:
tier base known/sec/armed Δ base first usable base write rows p1 known/sec/armed Δ p1 first usable (first / median) 100 0 / 12 / 15 103 ms 27 0 / 0 / 15 104 / 103.0 ms 150 0 / 12 / 15 156 ms 27 4 / 4 / 15 154 / 152.0 ms 500 0 / 12 / 15 503 ms 27 13 / 13 / 15 503 / 3.0 ms Reading, stated in both directions:
- Low tiers (100–150): the arms split the median. Phase1 does not switch early here (tier 100: 0 secondaries; tier 150: first switch only after ~3 estimator samples), so its per-block median equals the applied delay (103.0 / 152.0 ms); baseline's redirects pull the median down to 3–4 ms. Baseline is better on the median at low tiers; first-arrival values are within noise of each other (104 vs 103; 154 vs 156 ms).
- Mid/high tiers (≥ 300): both arms' medians are 3–5 ms and the difference moves to the tail — base-t500 p90 ≈ 303 / 502 ms at tiers 300/500 vs phase1 184 / 303 ms; phase1's max is 304 / 504 ms (the 504 at tier 500 is the residual full-length fallback; the 304 at tier 300 tracks the applied delay itself).
- Traffic trade, both directions: baseline's redirect replies show up as 27–30 write rows per window with already-known Δ = 0. Phase1 dispatches its secondaries early (typically ≤ ~60 ms), they arrive before the delayed peer's copy, and the old peer's copy then lands as the duplicate — already-known Δ 4–15 per window at tiers ≥ 150, i.e. roughly one duplicate block response per secondary, bounded at one per secondary per block. At tier 100 phase1 sends no secondaries (0/15) and records zero duplicates. All 28 windows reconcile at 15 ± 2 distinct blocks; neither arm drops blocks.
The honest outcome statement is therefore: at low tiers baseline wins the median (redirect rescue) while phase1 holds its gate; at mid/high tiers phase1 wins the tail and pays for it with bounded duplicate arrivals, while baseline pays the tail latency and carries zero counted duplicates.
5. Narrowed conclusions
- Switch coverage: unchanged from the round-3 reading.
- Timeout waiting: baseline's full-timeout waits are a p90-tail phenomenon, not the median; phase1 removes them except for ≤ 2 events per window, which run at full length as the designed fallback.
- The 100/150 ms no-switch behavior is remaining-time-gated by design; the estimator crosses the comparison threshold after ~3 samples at tier 150 in the quoted windows.
- Outcome: baseline keeps the better median at low tiers; phase1 improves the tail at mid/high tiers, paid for with bounded duplicate-arrival overhead (§4).
- The "1–2 samples to clamp" wording is qualified per §3 and will be corrected in the issue text.
6. Artifacts
To be linked in the PR description:
- Instrumentation commits:
acefb0717991(instrumentation/trace-counters),2a49bdb60808(instrumentation/trace-phase1). - Harness:
chain7-baseline.sh,chain7-phase1.sh, netem tier schedule, node configurations. - Reduction:
reduce_r3.py,REDUCTION-R3.md. - This addendum:
analysis-r4/—commits.md,dispatch-latency.{md,tsv},sequences.md,outcome-windows.{md,tsv},outcome-latency.{md,tsv},verify.md(30/30 + 28/28 checks). - Raw logs: the four canonical run directories.
- Baseline
Thanks for extracting the sequences and correcting the interpretation of the baseline label. The dispatch-time and remaining-time explanations clarify the earlier questions. Two measurement-definition issues still need resolving before relying on the outcome conclusions:
-
The already-known counters are not comparable. In baseline
acefb071,BLOCK_ALREADY_KNOWNis incremented whenblock.getNum() < headNum. In phase12a49bdb6, it requires a matching outstanding adv request and an exact block ID already known before processing. A duplicate at the current head is therefore not counted by the baseline. Please revise the inference that duplicate overhead occurs only in Phase 1, and compare response counts/bytes for the same tracked-block cohort using a common definition. Reconstruct this from retained logs where possible; otherwise align the instrumentation for a targeted comparison. -
Please verify the time origin in the outcome reduction. The native fetch field is
nowminus the responding peer's own request timestamp fromadvInvRequest, not necessarily the initial request timestamp. A secondary dispatched 40 ms after the initial request and answered in 2 ms gives roughly 42 ms of total waiting, not 2 ms. Please calculate each block's earliest response against one common initial-request timestamp. Taking the minimum peer-local fetch duration would omit the secondary-dispatch delay. Also distinguish receipt from successful processing when calling the result "first usable".
Please include per-window sample counts and per-block latencies alongside any quantiles. With roughly 15 blocks per window, descriptive p90 differences alone do not establish a repeatable tail-latency improvement.
One correction to the convergence wording: reaching 0.8 times the injected delay is not the same as reaching the timeout clamp. A warmed estimate can also clamp in one update; for example, old=4 ms, sample=2000 ms and timeout=200 ms. Please report the observed sample-to-clamp sequences separately.
For #6988, please link the actual analysis-r4 scripts, raw logs and corrected tables. Also document the observed reduction in secondary requests at low degradation, and the role of the unsampled channel-latency fallback, in the behavior-change section. Reserve claims about earlier first arrival or improved tails for the corrected timing results. No broader experiment matrix is needed.
I support continuing with #6988, with the corrected outcome evidence reviewed before merge.
-
Reply — measurement-definition corrections: already-known counts and per-block outcome latency
@waynercheung Both points are valid. The two arms counted already-known at different points (baseline:
block.getNum() < headNum; phase1: outstanding adv request match plus exact id contained before processing), so the earlier cross-arm comparison of that counter was not sound — I withdraw the inference that duplicate-arrival overhead existed only in Phase 1. The latency numbers had the same class of problem: they measured from the responding peer's own request timestamp instead of the common initial request. Both are corrected below, and the outcome latency is now split into first arrival and first usable.1. Correction 1 — already-known under a unified definition
Unified best-effort definition from the logs: for every tracked block (each
Set fetchBlockInfoline, i.e. each armed initial request), count same-id BLOCK responses arriving after the first-arriving response in the same window. A duplicate arrival is a necessary condition for already-known, so this is an upper-bound approximation of the phase1 counting point; the phase1 counter is itself documented in source as a lower-bound indicator, and the logs carry no outstanding-request markers, so the two are bounds from opposite sides rather than two measurements of the same quantity.Reconstruction vs counters, per window (dup = reconstructed duplicates, known Δ =
tron:block_already_knownPOST−PRE, sec Δ =tron:block_fetch_secondaryPOST−PRE; n = tracked blocks, 15 or 16, equals the armed delta in all 28 windows):tier baseline-t200 baseline-t500 phase1-t200 phase1-t500 100 13 (0, 13) 12 (0, 12) 0 (0, 0) 0 (0, 0) 150 12 (0, 12) 12 (0, 12) 11 (10, 11) 4 (4, 4) 200 12 (0, 12) 12 (0, 12) 7 (7, 7) 12 (12, 12) 300 15 (0, 15) 12 (0, 12) 14 (14, 14) 12 (12, 12) 500 15 (0, 15) 12 (0, 12) 15 (15, 15) 13 (13, 13) 1000 15 (0, 15) 15 (0, 15) 15 (15, 15) 14 (14, 14) 2000 15 (0, 15) 15 (0, 15) 13 (13, 14) 15 (15, 15) Three points. Under the unified definition the baseline arm also produces 12–15 duplicate arrivals per window, the same order as phase1 at the higher tiers — the duplicate traffic itself is not a Phase-1 phenomenon. The baseline counter stays 0 in all 14 windows because a duplicate arriving ~one tier-period after the first copy never satisfies
getNum() < headNum; that is a property of the counting point, not a behavior difference. On the phase1 side the reconstruction equals the known delta in 13 of 14 windows; the one gap (tier 150) is a 2 ms containment-visibility race, and the sec-delta exceeds the known delta by one at t200/2000 because that block's secondary response falls outside the log truncation and the POST snapshot. For completeness:block_fetch_secondarycounts secondary dispatch sends, and its equality with the reconstruction in all 14 baseline windows is an empirical one-request-one-response consequence of this testnet, not a definitional identity.2. Correction 2 — per-block outcome latency from the common initial request
Each block is measured from its earliest
Set fetchBlockInfoepoch (the common initial request; no block had a second Set in any window). First arrival is the firstFetch block successline for that id; first usable is the firstSuccess process blockline. Tables report first-usable latency; arrival medians sit typically 15–25 ms below usable medians across the 28 windows. n = 15 per window except two phase1-t200 windows at 16; per-block detail is in the linked tsv.First-usable ms (median / p90 / max):
tier baseline-t200 baseline-t500 phase1-t200 phase1-t500 100 64 / 112 / 122 54 / 117 / 118 121 / 128 / 217 124 / 129 / 170 150 64 / 169 / 173 52 / 167 / 185 57 / 173 / 178 170 / 183 / 192 200 80 / 222 / 224 68 / 223 / 230 170 / 220 / 224 56 / 227 / 238 300 61 / 240 / 262 75 / 323 / 330 48 / 184 / 261 63 / 224 / 337 500 60 / 234 / 263 50 / 512 / 520 68 / 119 / 255 64 / 348 / 534 1000 71 / 246 / 267 67 / 556 / 573 64 / 90 / 274 71 / 81 / 525 2000 61 / 239 / 268 67 / 535 / 571 56 / 87 / 262 59 / 137 / 535 Single-run per cell, so direction only — consistent across both runs: at the 100 ms tier phase1 is slower on median (121/124 ms vs 64/54 ms baseline) because the EWMA estimate reacts slowly to the sudden onset of injected delay and comparison failures keep the old peer for the early blocks of the window — the real cost of the conservative gate, not a switch failure. From the 500 ms tier upward the corrected numbers show the intended behavior: phase1 p90 falls to 119/90/87 ms against baseline 234–246/512–556 ms in the same tiers. (Rounded to whole ms here; the linked tsv carries one decimal where present.)
3. Sample-to-clamp sequences for the EWMA estimate
The instrumentation logs every estimator update as
updateFetchLatency peer=… oldMs=… sampleMs=… newMs=…; seeding is first-sample, blending is alpha 0.1 (new = (old*9 + sample)/10), clamped tofetchBlock.timeout(per run: 200 in t200, 500 in t500, per the config read-back). On your specific point, accepted: the steady-state estimate is not "0.8× the injected delay" — it converges toward the sample value and is bounded by the clamp. The windows show convergence-then-saturation:- t200 (clamp 200): steps from the first in-window sample to the clamp are 1 at tier 2000 (one-step:
oldMs=6, sampleMs=2003, newMs=200— the warm-up-seeded ~6 ms estimate jumps straight to the clamp), 3 at 1000, 5 at 500, 11 at 300; tiers 200 and below approach the injected value asymptotically and never reach the clamp in-window (tier 200 tops out at 158). - t500 (clamp 500): 3 steps at tier 2000 (206→385→500), 7 at 1000; tier 500 and below do not reach the clamp in-window (tier 500 tops out at 385).
- Once clamped, newMs stays at the clamp for the rest of every window; no fallback was observed (the injected peer's samples never drop back inside a window).
One-step clamp from the warm-up estimate is confirmed where the injected delay is at or above roughly ten times the seeded estimate and the clamp is tight; at lower tiers the estimate glides toward the injected delay over ~10 samples — the stickiness behind the tier-100 median above.
4. Artifacts
The reduction script
analyze_r5.pyand its outputs (per-block detail tsv, already-known table, sequences) will be linked in #6988; the first-arrival and tail conclusions in the earlier replies are superseded by the corrected per-block numbers above. I am not requesting a larger experiment matrix — the current 28 windows answer both points within the stated limitations.- t200 (clamp 200): steps from the first in-window sample to the clamp are 1 at tier 2000 (one-step:
Thanks, this addresses the measurement-definition concerns in principle. The reconstructed responses, common-origin timing, and actual sample-to-clamp sequences are the right corrections. No broader experiment matrix is needed.
The remaining work can be completed in #6988:
-
Please publish
analyze_r5.py, the per-block results, and the raw logs/configurations needed to reproduce the tables, replacing the current link to the earlier summary. Label the timing origin as tracking-arm time:Set fetchBlockInfooccurs after the initial send call. Keep the reconstructed duplicate-response counts distinct from the runtime already-known counter, flag the truncated response explicitly, and limit overhead claims to response counts unless bytes were also measured. -
Please update the issue and the PR's behavior-change section with the observed trade-off. At the 100 ms tier, Phase 1 sends fewer secondary requests but has higher first-usable medians: 121/124 ms versus 64/54 ms. Document the role of estimator adaptation and the unsampled candidate fallback. Keep the timeout groups separate, and describe the higher-tier p90 improvements as observations from these single runs, not a general performance guarantee.
-
Replace the remaining blanket "1-2 samples" wording with the measured per-tier clamp counts. Please also avoid the "roughly ten times the seeded estimate" rule: a one-update clamp depends on the sample, the previous estimate, and the timeout together.
The structural benefits remain a sound reason to pursue the change; it does not need to be faster in every regime. Once the artifacts and documentation are available, we can finish the experimental review in #6988 and assess the recorded trade-off alongside the code and tests before merge.
-
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsNo status
Summary
java-tron maintains two parallel metrics stacks with independent switches: a legacy Dropwizard-based implementation (
org.tron.core.metrics, gated bynode.metricsEnable) exposed only through the gRPCMonitorApi.getStatsInfoendpoint and the HTTP/monitor/getstatsinfoservlet, and the Prometheus stack (org.tron.common.prometheus) that has become the standard monitoring path.This proposal stages the decommissioning of the legacy stack together with
MonitorApiover two releases, leaving Prometheus as the single supported monitoring backend. Phase 1 (next release) migrates the functional consumer, adds fetch-behavior counters that are retained long-term, and marks the legacy APIs deprecated while keeping them fully functional. Phase 2 (the agreed major release) removes the legacy implementation, its configuration chain, the Dropwizard dependency, and the protobuf definitions. The proposal also adds atron:node_info{version, genesis_block_id}info metric so that node information stays easy to observe.This is the implementation issue for item 7 of the tracking issue #6921.
Problem
Motivation
Prometheus has become the standard monitoring solution for java-tron (see #6590 and the tron-docker
metric_monitorreference stack). Keeping a second, non-Prometheus implementation alongside it means duplicated instrumentation and extra maintenance, and it misleads operators: enablingnode.metricsEnablewithout Prometheus produces no scrapeable output at all. The legacy endpoints have no known production consumers.Current State
MetricRegistryand is served only viaMonitorApi.getStatsInfo(gRPC) and/monitor/getstatsinfo(HTTP). Most of its fields already have Prometheus equivalents.FetchBlockService.getPeerTop75) consumes the per-peernet.latency.fetch.block.<peerIP>75th-percentile histogram as functional failover input. These histograms are written viahistogramUpdateUnCheck— bypassing the metrics-enable gate — use one unbounded key per peer IP that is never cleaned on disconnect, and mix latency samples from different block-fetch paths, so the choice of metrics backend silently dictates scheduling behavior.Limitations or Risks
node.metricsEnable=truewho skip the deprecation window must not silently lose monitoring: the Phase 2 release retains a tombstone warning when the key is still present.Proposed Solution
Proposed Design
The change is staged over two releases.
Phase 1 (next release) — migrate the functional consumer and deprecate the legacy APIs.
Fetch-block peer selection. Fetch-block peer selection switches from the per-IP P75 histogram to a bounded, per-connection estimate of actual block-fetch duration. Each
PeerConnectionholds an explicit unsampled state plus avolatile longEWMA. While unsampled, reads fall back to the libp2p channel RTT (Channel.getAvgLatency(), the same signalPeerManager.sortPeersalready uses); the first real fetched-block measurement replaces the fallback instead of entering the EWMA, and smoothing starts from the second measurement. The steady-state weight is a fixedα = 0.1((ewma * 9 + last) / 10) expressed as a named constant with its rationale documented; a warm-up running mean is deliberately not used. Estimates are clamped tofetchBlockTimeout, with an explicit saturation gate that requires a strictly better candidate and an unconditional hard-timeout branch once the wall-clock fetch budget is exhausted. Because the field lives onPeerConnection, it is released with the connection — no disconnect bookkeeping and no unbounded per-IP keys. The per-IP histogram read and write sites are removed. Global fetch-latency observability stays available through the existing unlabeled Prometheus histogram (a per-peer Prometheus label would reintroduce the unbounded-cardinality problem in the TSDB).Legacy APIs stay functional. The gRPC
Monitorservice and the HTTP/monitor/getstatsinfoendpoint remain registered and keep serving as before. Both are marked deprecated (option deprecated = trueonservice Monitorandmessage MetricsInfo), documented as deprecated in release notes and docs, and accompanied by a startup deprecation warning whennode.metricsEnableis present plus a process-once WARN on the first invocation of either deprecated API. A legacy-field-to-Prometheus mapping table, including fields without an equivalent, is published in the PR description, release notes, and the English/Chinese documentation.Node information. Add a
tron:node_info{version, genesis_block_id}info metric (collector base nametron:node, queried astron:node_info):versionpreserves the node version previously carried only by the legacy payload and not currently exposed to Prometheus.genesis_block_idis the full genesis block hash — the canonical TRON chain identifier. Mainnet, Nile and private networks each have distinct values, so dashboards and PromQL can tell at a glance which network a node belongs to (e.g.tron:node_info{genesis_block_id="..."}).instancelabel already identifies the node.Fetch-behavior counters (retained long-term). Add three unlabeled, monotonically increasing Prometheus counters:
tron:block_fetch_armed— incremented whenFetchBlockServicearms a fetch tracking.tron:block_fetch_secondary— incremented when a secondary fetch is sent.tron:block_already_known— incremented for a matching outstanding adv request whose exact block ID is already known before processing that response. A block below the current head is not necessarily already known; it may be an unseen fork block, so the counter requires a matchingadvInvRequest.remove(item)together with the exact block ID already being known. It is a best-effort signal: concurrent arrivals may be missed, and it does not establish secondary-fetch attribution. Exact attribution remains in the experiment harness.These counters describe fetch behavior that remains after the legacy stack is removed. They are not coupled to the Dropwizard cleanup and are retained beyond Phase 2; removing them later would be a separate observability decision. Normalized rates (secondary-fetch ratio, already-known response rate) are derived in PromQL from the counters rather than precomputed on the node.
Phase 2 (the agreed major release) — remove the legacy stack. Remove
MetricsUtil, the legacy metric managers and DTOs, the gRPCMonitorApiregistration, the/monitor/getstatsinfoservlet route, all legacy write sites, thenode.metricsEnableconfiguration chain, and the Dropwizard dependency, following theWalletExtensionstaging precedent from #6921. Remove the protobuf definitions (service Monitorinapi.proto,message MetricsInfoinTron.proto); clients calling the removed endpoints receiveUNIMPLEMENTED/ 404. Whennode.metricsEnableis still present, emit a tombstone warning so operators who skip Phase 1 do not silently lose monitoring. The three fetch-behavior counters andtron:node_inforemain./monitor/getnodeinfois node info, not metrics, and is unaffected.Key Changes
common(new info metric and fetch-behavior counters),framework(migration in Phase 1; removal in Phase 2), build files (Dropwizard dependency removed in Phase 2).node.metricsEnableworking and warns when it is present; Phase 2 removes the configuration chain and leaves a tombstone warning for a residualnode.metricsEnable.node.metrics.prometheus.enable/.portkeep their semantics throughout.Monitor.GetStatsInfoand HTTPGET /monitor/getstatsinfofunctional but deprecated; addstron:node_info{version, genesis_block_id}and the three fetch-behavior counters. Phase 2 stops serving the legacy APIs and removes their proto definitions.Impact
Compatibility
Monitor.GetStatsInfoand HTTPGET /monitor/getstatsinforemain functional; both are marked deprecated in the protos and docs with warnings. In Phase 2 they stop being served (UNIMPLEMENTED/ 404) and the protobuf definitions are removed, following principle 2 of Tracking: code refactor and cleanup #6921 and theWalletExtensionstaging precedent. (Today the gRPC endpoint is only registered whennode.metricsEnable=true, which defaults tofalse, and the HTTP endpoint serves mostly empty metric fields when the switch is off.)fetchBlockTimeout, instead of the raw 75th percentile of the per-IP histogram. Semantics stay close to the old intent (actual fetch duration) while fixing its defects: mixed request sources, no decay, unbounded keys. The unsampled state falls back to the channel RTT and the first real fetch replaces it, so a newly connected peer is never ranked with a placeholder observation (previously an empty histogram silently ranked a peer as fastest — pinned by unit tests). One intentional behavior change is listed explicitly per principle 1 of Tracking: code refactor and cleanup #6921: previously a candidate whose P75 exceededfetchBlockTimeoutwas filtered out; with the clamped EWMA a saturated candidate stays eligible and may receive the secondary request at the hard timeout when no better candidate exists. This is a deliberate liveness improvement and is documented and tested.node.metricsEnable = truenode.metrics.prometheus.enable = truenode.metrics.prometheus.port = 9527(default)The key is ignored silently by config parsing in Phase 2; the release notes must call out this migration, and the tombstone warning must fire when the key is still present.
Test Plan
α = 0.1, clamp, saturation gate, hard-timeout branch, boundary values, and reconnect isolation; tests fortron:node_infoand the three fetch-behavior counters, including the precisetron:block_already_knowndefinition.tc netem), and report how many additional per-peer samples are needed for the ranking to flip; thedevelopbaseline runs the same harness and topology with the counters cherry-picked onto an instrumentation-only branch so both arms expose identical metrics. Report the normalized secondary-fetch rate and already-known response count; primary/secondary attribution comes from the harness. Measured replacement for the earlier one-or-two-sample phrasing: atfetchBlockTimeout = 200the estimate reaches its clamp within 1-2 in-window samples and the first switch lands on block 2-3 of a degradation window; atfetchBlockTimeout = 500sub-budget degradation does not saturate and only the comparison path fires (decision traces linked from the PR).Rollback
Phase 1: revert the PR commits; the protobuf definitions are unchanged (only marked deprecated), so the previous behavior is restored without protocol or config migration. Phase 2: revert reintroduces the legacy endpoints and the Dropwizard dependency, and requires restoring the removed proto definitions.
References
Additional Notes