You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hop.apache.org by "mattcasters (via GitHub)" <gi...@apache.org> on 2026/09/23 18:31:16 UTC

[PR] Issue #8546 : Keep pipeline metrics updating after a Kafka consumer restart (hop)

mattcasters opened a new pull request, #8560:
URL: https://github.com/apache/hop/pull/8560

   ## Summary
   
   Fixes #8546.
   
   Stopping a pipeline sets the stopped flag before the engine has finished. For a Kafka consumer that window is long enough to start again while the previous engine is still leaving `poll()`. The previous engine's finished listener then stopped the canvas and metrics timers of the run now on screen. Batches kept showing up in the log, and the grid stayed at 0 until the next stop refreshed it once.
   
   The Kafka batch log made that look like a metrics bug of its own. `Finished processing (I=…, R=…, W=…)` is one sub-pipeline iteration, and those counters were cleared immediately afterwards when no execution information location was set. `Number of rows read` was the injector buffer size, not a grid metric. The parent transform only increments Input.
   
   - Refresh timers are bound to the engine currently shown. A finished or stopped listener from the previous pipeline or workflow cannot stop the new run.
   - Start is refused until the previous pipeline has finished, using the existing "do not start twice" dialog. `isRunning()` is unchanged, so the toolbar behavior is the same.
   - The Kafka consumer keeps cumulative sub-pipeline metrics and logs `Kafka consumer batch of N record(s), cumulative input M`. Read and Written on the Kafka transform stay 0 unless result fields are mapped back. Mapping and Beam still clear counters per iteration.
   
   ## Test plan
   
   - [x] `./mvnw -pl ui test -Dtest=ExecutionGuiSessionTest`
   - [x] `./mvnw -pl plugins/transforms/kafka -am test -Dtest=KafkaConsumerInputTest -Dsurefire.failIfNoSpecifiedTests=false`
   - [ ] Stop a running Kafka consumer, start it again before the first engine has fully exited, and confirm Input keeps increasing while batches are logged
   - [ ] Drill into the running sub-pipeline and confirm its transform metrics accumulate instead of returning to 0 after each batch
   - [ ] Stop and restart a workflow with a slow action and confirm the canvas keeps redrawing


-- 
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: issues-unsubscribe@hop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Issue #8546 : Keep pipeline metrics updating after a Kafka consumer restart (hop)

Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters merged PR #8560:
URL: https://github.com/apache/hop/pull/8560


-- 
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: issues-unsubscribe@hop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Issue #8546 : Keep pipeline metrics updating after a Kafka consumer restart (hop)

Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on PR #8560:
URL: https://github.com/apache/hop/pull/8560#issuecomment-5847788129

   Thanks for the review. Addressed in 6c3a232c4d.
   
   1. and 2. Start and debug use `!isRunning()` again. A successful Beam run and a failed preparation never set finished, so that check blocked the next Run. The session guard is what keeps a stale listener off the new timers.
   3. and 5. `checkPipelineEnded`, `pipelineFinished`, and the workflow equivalents run before the session lock is taken. Only `stopRedrawTimer()` stays inside `stopIfCurrent`, and it does not take the graph lock or open a dialog.
   4. Removed `setClearingMetricsPerIteration(false)`. Nothing called `buildExecutionSummary()` after the Kafka call went away, so the flag and that method are deleted. Sub-pipeline counters are no longer cleared because nothing wipes them.
   6. `scheduleWhileCurrent` cancels the timer on the first tick that sees a newer engine. Pipeline and workflow redraw, and the metrics grid, all go through it.
   The workflow listeners are registered before the execution thread and the redraw timer. `adopt` returns the engine and generation together. The first-row date is set once per batch, before the record loop.


-- 
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: issues-unsubscribe@hop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Issue #8546 : Keep pipeline metrics updating after a Kafka consumer restart (hop)

Posted by "hansva (via GitHub)" <gi...@apache.org>.
hansva commented on PR #8560:
URL: https://github.com/apache/hop/pull/8560#issuecomment-5814173097

   
   ### Must fix
   
   1. **Beam pipelines can't be run again** (`HopGuiPipelineGraph.java:6083` and `:6290`)
      The start/debug check changed from `!isRunning()` to `pipeline == null || pipeline.isFinished()`. `BeamPipelineEngine` only calls `setFinished(true)` when the run FAILED. The DONE, STOPPED and CANCELLED cases set `running = false` and the status, but never `finished`. After any Beam run that completes normally, pressing Run shows "This pipeline is already running" every time until the tab is closed.
   
   2. **Failed preparation probably blocks retries too**
      When preparation fails, `Pipeline.flagPreparationFailure` calls `stopAll()`, which leaves the engine stopped. Please check whether `stopAll()` ever sets `finished`. If it doesn't, then after a refused transform or an unreachable remote server, Run is enabled but every retry says "already running". The session guard already handles stale listeners, so the stricter check adds nothing. Going back to `!isRunning()` fixes both 1 and 2.
   
   3. **The GUI can freeze: lock-order inversion** (`HopGuiPipelineGraph.java:6533`)
      - The finished listener takes the session lock and then calls `checkPipelineEnded()`. After a preview/debug run with zero breakpoint hits, that calls `showLastPreviewResults()`, which is `synchronized` on the graph.
      - `start()` and `debug()` are `synchronized` on the graph and take the session lock through `setDisplayedPipeline` → `adopt`.
   
      If a preview run finishes just as the user presses Run, each thread waits for the other, permanently. Before this change there was only one lock, so this couldn't happen. Suggested fix: take the session lock only to check that the run is still current, then call `checkPipelineEnded()` and the rest after releasing it.
   
   ### Should fix
   
   4. **The Kafka fix does the same thing twice** (`KafkaConsumerInput.java:214`)
      The change removes the only call to `buildExecutionSummary()`, and that method is the only place `clearingMetricsPerIteration` is read. So `setClearingMetricsPerIteration(false)` now does nothing, and the comment "Mapping and Beam still clear per iteration" is wrong because nothing else calls it. Pick one approach:
      - Keep the call and the flag. The sub-pipeline keeps its per-batch summary log lines.
      - Remove the call and delete the now-unused flag and method.
   
   5. **Listener code runs while holding the session lock** (`HopGuiPipelineGraph.java:6200`)
      `pipelineFinished` runs extension points and may show an `ErrorDialog`. The GUI thread takes the same lock on every redraw tick. The class Javadoc says the runnable "must not wait on the GUI thread", but plugin code in extension points can't be held to that.
   
   6. **Old redraw timers are never cancelled** (`HopGuiPipelineGraph.java:6659`, `HopGuiWorkflowGraph.java:6049`)
      A timer whose engine is no longer current skips its work but keeps running. The old engine's finished listener, which used to cancel it, is now ignored, so one Timer thread can be leaked per occurrence. Calling `cancel()` when `isCurrent()` is false fixes it.
   
   ### Smaller points
   
   - **Workflow `start()` has a timer race** (`HopGuiWorkflowGraph.java:5467`). The thread and the redraw timer start before the finished and stopped listeners are added. A trivial workflow that finishes in that window leaves the timer running. This problem already existed, but this change is where it gets fixed.
   - **The engine and generation are read in two steps.** Having `adopt()` return both together as one snapshot would close that gap and remove the three pass-through methods on the graph (`currentExecutionGeneration`, `isCurrentExecution`, `runIfCurrentExecution`).
   - **Duplicated code.** The timer-install logic is copied between the pipeline and workflow graphs, and the finished-listener lambda appears in both `startThreads` and `attachToRunningPipeline`. A shared helper would keep future fixes (such as 6) in one place.
   - **The Kafka first-row-date check runs for every record** (`KafkaConsumerInput.java:361`). Moving it above the loop runs it once per batch.
   
   ### Suggested way forward
   
   Keep the Kafka part once 4 is tidied up. For the GUI part, go back to `!isRunning()` to fix 1 and 2, and move the listener's work out from under the session lock to fix 3.
   


-- 
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: issues-unsubscribe@hop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org