Doris-Breakwater commented on issue #67777: URL: https://github.com/apache/doris/issues/67777#issuecomment-5614251830
Breakwater-GitHub-Analysis-Slot: slot_4683a0b5eeb0 ### Initial triage **Judgment:** this is a valid and actionable BE performance issue. The periodic 15-second stall is strongly supported by both the runtime evidence and the exact `7126cf65d96` source. The rare 2–15 second HTTP-wide stalls expose a real architectural risk, but their attribution to stream load is not yet proven. The issue currently has no labels or assignee; BE/performance and metrics/observability categorization would be appropriate if those labels exist. #### Verified facts - `Daemon::calculate_metrics_thread()` invokes `trigger_all_hooks(true)` before each 15-second wait. `MetricRegistry::trigger_all_hooks()` holds the registry mutex while it invokes every entity hook, and `to_prometheus()`, `to_json()`, and `to_core_string()` acquire the same mutex. - The Doris server-entity hook calls `_update_process_fd_num()`. In this revision it applies `directory_entry::is_regular_file()` to every `/proc/self/fd` entry. Because those entries are symlinks, this follows each target and is consistent with the reported one-`newfstatat`-per-FD stack and the measured cost at approximately 197k FDs. - `to_prometheus()` keeps the registry mutex through metric collection and string serialization, so concurrent scrapes of the same registry are serialized. Setting `enable_metric_calculator=false` is not a clean mitigation: `trigger_hook_unlocked(false)` then executes the same hooks in the scrape path. - The same metrics locking and FD enumeration remain present in the current local `branch-4.1` and `master` snapshots, so this is not already resolved there. - `EvHttpServer` runs one `event_base_dispatch()` per worker, and its request/chunk callbacks invoke `handler->handle()` / `on_chunk_data()` inline. Stream load has several blocking points on that thread: header-time begin/planning RPCs, pipe back-pressure, `body_sink->finish()`, `ctx->future.get()`, and commit/pre-commit RPCs. Therefore a stream-load request can stall its owning HTTP event loop. One important design caveat: copying the entity list under `MetricRegistry::_lock` and then releasing that lock is not sufficient by itself. Hooks still run while holding the corresponding `MetricEntity::_lock`, and the exporter also takes that entity lock, so a scrape can merely move from waiting on the registry mutex to waiting on the server-entity mutex. Any lock refactor should prove that slow hook execution is decoupled from both registry and exporter-critical sections, with callback/entity lifetime kept safe. #### Not yet verified The observation that `/api/health` stalls while `calculate_metrics_thread` sleeps rules out the metrics-registry lock for those particular events. It does **not** establish that stream load caused them. With 128 event loops and about 22 in-flight loads, simultaneous multi-second delays on independent probes could also involve worker assignment/connection reuse, CPU scheduling or cgroup throttling, or another process-wide pause. The code proves the possibility of event-loop blocking, not the reported causal chain. #### Evidence needed for the rare stalls 1. Capture user-space stacks plus state/wchan for **all** `EvHttpServer` workers during a >2-second event, and summarize how many are in `future.get()`, FE/MS commit or planning RPC, `StreamLoadPipe::_put_cond`, runnable state, or `event_base_dispatch()`. 2. Record `time_connect`, `time_starttransfer`, and `time_total` for `/metrics`, `?type=core`, and `/api/health`, and state whether each probe uses a fresh connection or keep-alive. 3. Provide the matching BE stream-load completion log fields (`load_cost_ms`, receive/read/write cost, and commit/publish cost) and relevant FE/MS RPC latency for the same timestamps, with sensitive values removed. 4. Run one controlled comparison with the high FD count unchanged but stream loads paused. If the 15-second spikes remain while the rare multi-second spikes disappear, that would isolate the two mechanisms much more convincingly. A short `vmstat`/run-queue trace and cgroup CPU-throttling deltas over the same window would help exclude a process-wide scheduling cause. #### Recommended next steps 1. Treat the periodic metrics stall as confirmed and fix the FD hook first: count directory entries without following every target (while accounting for the enumeration directory FD if exact counting is required). This also makes `process_fd_num_used` include sockets/pipes rather than only descriptors whose targets resolve as regular files. Add a focused correctness test covering regular files, pipes, and sockets, plus a high-FD cost benchmark or regression test. 2. Follow with a metrics-locking change and a concurrency regression test using an intentionally slow hook. The acceptance condition should be that a slow hook does not delay a default `/metrics` snapshot and that parallel scrapes do not queue behind full serialization. A prebuilt/safely published snapshot is one viable direction; any entity-list-copy approach must also address the entity-lock caveat above. 3. Keep the rare HTTP-worker stalls as a separate work item (or at least a separate commit) until worker-stack evidence identifies the blocking stage. If confirmed, offloading blocking stream-load phases is the systemic fix; a dedicated metrics listener is a narrower observability safeguard, not a substitute for resolving event-loop blocking. The first two changes are independently justified by the current report. The broader HTTP change needs the additional capture above before selecting an implementation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
