Conversation
The periodic move_results_to_core() event drained the async submit pipeline with a non-blocking flush. That never gets a job out: run_tasks() on a client with GEARMAN_CLIENT_NON_BLOCKING returns GEARMAN_IO_WAIT and makes no progress unless the caller waits for the socket to become ready, so calling it again a second later just returns GEARMAN_IO_WAIT again. The pending count grows and nothing is ever delivered. Where checks come in fast enough this is invisible, because the next submit drives the pipeline within microseconds. Where they do not, there is no next submit for minutes and the periodic flush is the only thing that could deliver them -- so the checks are simply never executed. Reproduced with 10 services on a 60s interval against an unpatched naemon 1.5.2: over a 90 second window not one of them had last_check advance, and it stayed 0 for the whole run. The same setup on v5.2.5 advances all ten. With this change all ten advance again, at a check_latency of 0.5-0.9s, which is what v5.2.5 shows as well. Add gm_drain_submits() for that one job and call it from the event. It is a blocking flush, bounded by gearman_connection_timeout (5s by default) -- the same bound the per-job gearman_client_do_background() had before the pipeline existed, and now once per second rather than once per check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WY8mbGLNkt5eQfc57cTnZ5 Signed-off-by: nook24 <info@nook24.eu>
…g_data Two defects found while chasing the delivery bug above. gm_flush_submits() mapped everything but GEARMAN_SUCCESS and GEARMAN_IO_WAIT to GM_ERROR. A blocking flush that has already handed its tasks over then polls with nothing left to drive until the client timeout expires and returns GEARMAN_TIMEOUT, so a submit that was in fact delivered got reported as failed. In add_job_to_queue() that turns into rc = GEARMAN_ERRNO and the caller cancels the check; handle_svc_check() unsets is_executing and returns NEBERROR_CALLBACKCANCEL for a check that is already on its way. The payload slot was taken with gm_pending_data[gm_pending_submits++] before the flush that is supposed to free the slots. When a flush does not release them -- which the GEARMAN_TIMEOUT case above makes reachable -- the next submit writes one past the end of the array. Check the capacity before queuing the task instead, drain synchronously if it is exhausted, and give up on the job rather than corrupting memory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WY8mbGLNkt5eQfc57cTnZ5 Signed-off-by: nook24 <info@nook24.eu>
The async tests force a blocking gm_flush_submits() right after submitting, so they never exercise leaving a job queued for something else to deliver -- which is exactly what the periodic event in the NEB module does, and exactly where the delivery bug was. Nothing in t/ would have caught it. Add test_async_drain_only(): submit one job and let nothing but gm_drain_submits() -- the same call the event makes -- deliver it, then require the queue to drain. Switching that function back to a non-blocking flush fails this test. The existing blocking flush assertion was ok(TRUE, ...) with the real return value only printed as a diagnostic, so it could not fail. It was in fact returning GM_ERROR the whole time, which is what put the GEARMAN_TIMEOUT handling in the previous commit on the trail. Assert the return value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WY8mbGLNkt5eQfc57cTnZ5 Signed-off-by: nook24 <info@nook24.eu>
Owner
|
thanks, i manually merged parts of this PR (b238340) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #192.
The problem
move_results_to_core()drains the async submit pipeline once per second with a non-blocking flush. That never gets a job out.gearman_client_run_tasks()on a client withGEARMAN_CLIENT_NON_BLOCKINGreturnsGEARMAN_IO_WAITand makes no progress unless the caller waits for the socket to become ready, so calling it again a second later just returnsGEARMAN_IO_WAITagain. The pending count grows and nothing is ever delivered.Where checks come in fast enough this is invisible, because the next submit drives the pipeline within microseconds. Where they do not, there is no next submit for minutes, and the periodic flush was supposed to be the thing that delivers them.
The 64-job
GM_MAX_PENDING_SUBMITSbuffer is not involved. It is only a ceiling that forces a blocking flush, never a threshold that has to be reached before anything is sent.