You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@hertzbeat.apache.org by "hengyuss (via GitHub)" <gi...@apache.org> on 2026/07/11 10:30:34 UTC
[PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
hengyuss opened a new pull request, #4196:
URL: https://github.com/apache/hertzbeat/pull/4196
## What's changed?
fix #4194
<!-- Describe Your PR Here -->
Allow all priority timeout metrics to execute dispatchCollectData and add staggered delay to avoid thread pool overload
## Checklist
- [x] I have read the [Contributing Guide](https://hertzbeat.apache.org/docs/community/code_style_and_quality_guide)
- [x] I have written the necessary doc or comment.
- [ ] I have added the necessary unit tests and all cases have passed.
## Add or update API
- [ ] I have added the necessary [e2e tests](https://github.com/apache/hertzbeat/tree/master/e2e) and all cases have passed.
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "zqr10159 (via GitHub) via notifications" <no...@hertzbeat.apache.org>.
zqr10159 commented on PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#issuecomment-6546025907
The current dispatcher already sends timeout results for every metric priority through the same once-only completion path as normal results. This covers the remaining production change here, so I’m closing it as superseded. Thanks for identifying the nonzero-priority timeout stall.
https://github.com/apache/hertzbeat/blob/b211c9aed1ce4ec97d162abddf30b68aa30b0b3f/hertzbeat-collector/hertzbeat-collector-collector/src/main/java/org/apache/hertzbeat/collector/dispatch/CommonDispatcher.java#L179-L183
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "Duansg (via GitHub)" <gi...@apache.org>.
Duansg commented on code in PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#discussion_r3565973272
##########
hertzbeat-collector/hertzbeat-collector-collector/src/test/java/org/apache/hertzbeat/collector/dispatch/CommonDispatcherTest.java:
##########
@@ -0,0 +1,193 @@
+package org.apache.hertzbeat.collector.dispatch;
Review Comment:
add license header
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "hengyuss (via GitHub)" <gi...@apache.org>.
hengyuss commented on PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#issuecomment-4951488187
i will remove the delay handle and open a new issue to resolve worker pool accumulate usage
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "tomsun28 (via GitHub)" <gi...@apache.org>.
tomsun28 commented on PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#issuecomment-5009906352
If we make this change, could it result in an unmanageable number of tasks? When a task is deemed unavailable, is there no longer a need to retry its other subtasks?
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "hengyuss (via GitHub)" <gi...@apache.org>.
hengyuss commented on PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#issuecomment-5010118889
1、the judgement of priority == 0 introduce by https://github.com/apache/hertzbeat/issues/66，at first it's same as this changed. and alert already is different now. but i can‘t estimate the number of tasks indeed currently.
2、it's true that no longer a need to retry its other substasks if a task is deemed unabailable. But the function monitorCollectTaskTimeout is guarantee the bottom line for task. Doesn't that mean the availability task with priority 0 has already been passed?if the program has run up to here.
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "Duansg (via GitHub)" <gi...@apache.org>.
Duansg commented on PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#issuecomment-5015399955
> If we make this change, could it result in an unmanageable number of tasks? When a task is deemed unavailable, is there no longer a need to retry its other subtasks?
@tomsun28 @hengyuss That's right; simply removing the `priority == 0` check isn't safe in and of itself, but I still think we need to remove this validation. Regarding the background of this PR, here's my take:
1. Would tasks grow unmanageably?
- No task repetition: The timeout dispatch only advances the task’s `priorMetrics` state machine; the next-level metric it triggers is precisely what should be executed in that round. `cyclicJob` only restarts after the current round is complete, so rounds are never repeated. Additionally, since #4169/#4201, the timer round mechanism has been fully safeguarded.
- Issue 1: Currently, there is no mechanism to cancel a worker that remains suspended (e.g., due to I/O blocking). Once this check (for `priority == 0`) is removed, the catch-all thread will continue to reschedule the worker in every round; therefore, a metric that never returns will leak a permanently stuck worker task in every round until the thread pool is exhausted;
- Issue 2: This change will exacerbate an existing race condition. For example, a task that eventually completes after timing out (>4 min) will still trigger a second call to `dispatchCollectData`
2. When a task is deemed unavailable, do we still skip its other metrics?
- The current logic is that if an availability collection with priority 0 fails or times out, `isAvailableCollectFailed` will still cause the entire round to terminate early, and none of the tasks below it will be scheduled. The only change in behavior occurs when the availability collection succeeds but a non-availability metric is suspended: currently, this causes the monitor to become permanently stalled, because `cyclicJob` is only restarted within `dispatchCollectData`, and the system does not schedule tasks for suspended metrics.
- Assumption: Suppose the task does indeed resume after 30 minutes. During those 30 minutes, there is no data, no TIMEOUT events, and no alerts; the page displays what appears to be normal, but outdated data. “Waiting for it to recover on its own” effectively leaves the decision on when to detect the delay up to the party responsible for the failure. While this may not matter to the data collector, it is disastrous for those using the monitoring system.
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "Duansg (via GitHub)" <gi...@apache.org>.
Duansg commented on code in PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#discussion_r3566391691
##########
hertzbeat-collector/hertzbeat-collector-collector/src/main/java/org/apache/hertzbeat/collector/dispatch/CommonDispatcher.java:
##########
@@ -182,6 +187,10 @@ private void monitorCollectTaskTimeout() {
log.error("[Collect Timeout]: \n{}", metricsData);
if (metricsData.getPriority() == 0) {
dispatchCollectData(metricsTime.timeout, metricsTime.getMetrics(), metricsData);
+ } else {
+ long delay = (long) metricsData.getPriority() * DELAY_PER_PRIORITY;
Review Comment:
Isn't this delay unnecessary? The `monitorCollectTaskTimeout` method triggers serial execution.
If a collection task times out or hangs, the fallback handling can cause the `worker pool` to accumulate usage. I think this is where we really need to implement backoff—what do you think?
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "Duansg (via GitHub)" <gi...@apache.org>.
Duansg commented on PR #4196:
URL: https://github.com/apache/hertzbeat/pull/4196#issuecomment-5015445431
@tomsun28 But I think we should adjust our approach here, rather than simply removing this validation.
For example:
1. The worker thread and the timeout monitor thread compete for exclusive access via CAS, ensuring that the result is distributed only once; the losing party silently backs off.
2. Adjust scheduling priority and interrupt the running worker thread. Note: In the default virtual thread pool, blocking socket I/O is interruptible (JEP 444), so stuck HTTP/JDBC/Redis read operations will be physically released at the timeout, rather than causing a resource leak.
3. The work thread must be mutually exclusive with the timeout monitoring thread to ensure that late interrupts never affect threads that have already switched to processing other tasks;
4. A mechanism for rolling back after consecutive timeouts (for example, skipping N rounds for a metric after M consecutive timeouts) will further reduce the number of repeated 4-minute waits caused by a single persistently suspended metric;
> Under this logic, in an extreme scenario, the worst-case scenario would be:
→ p0 Success
→ p1 Start (Response Blocked) ── 4 min ──→ Fallback: Discard p1, resend TIMEOUT, start p2
→ p2 (response blocked) ── 4 min ──→ Discard p2, resend TIMEOUT, start p3
→ p3 (response blocked) ── 4 min ──→ Discard p3, resend TIMEOUT, start p4
→ p4 (response blocked) ── 4 min ──→ Discard p4, resend TIMEOUT, end this round → cyclicJob → Start over from the beginning in the next round
Proceed sequentially, advancing one layer every 4 minutes; at any given time, only one layer is active, provided that p0 succeeds. If p0 fails first, maintain the existing design—the top layer will short-circuit—and simply proceed to the next round.
To summarize, the design goals are as follows: The fallback thread can advance the state machine without relying on the response status of a deadlocked task. Interrupts are used to free up resources as much as possible, while fallback dispatch ensures bounded recovery.
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org
Re: [PR] fix: dispatch timeout metrics for all priorities with staggered delay [hertzbeat]
Posted by "zqr10159 (via GitHub) via notifications" <no...@hertzbeat.apache.org>.
zqr10159 closed pull request #4196: fix: dispatch timeout metrics for all priorities with staggered delay
URL: https://github.com/apache/hertzbeat/pull/4196
--
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: notifications-unsubscribe@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@hertzbeat.apache.org
For additional commands, e-mail: notifications-help@hertzbeat.apache.org