beatum commented on PR #8544:
URL: https://github.com/apache/hop/pull/8544#issuecomment-6093032549
Thanks again for your time and for the detailed review. I really appreciate
the thoughtful suggestions.here’s a summary of the latest updates:
## Retry-loop fixes
Both retry-loop issues have been fixed in `PipelineExecutor`.
- `executeWithRetries()` now uses the parsed `waitTimeout` **for each
individual attempt**, rather than sharing one total timeout budget across all
retries. This restores the intended behavior where **Stop after** applies
separately to each execution attempt.
- Added stop-aware retry handling. The executor now checks `isStopped()`
before starting each attempt and waits for the retry delay through
`waitRetryDelay(...)` in short 100 ms intervals. This allows it to exit
promptly when the transform is stopped, instead of sleeping through the full
delay or starting another child execution.
Updated `PipelineExecutorTest` with the following tests:
- `executeWithRetriesUsesConfiguredStopAfterForEachAttempt`
- `executeWithRetriesDoesNotStartAnotherAttemptAfterTransformStop`
## Expanded-number support
`PipelineExecutor` now parses both retry-related fields with expanded-number
support, matching the dialog contract:
```java
retryAttempts = Const.toIntExpanded(resolve(meta.getRetryAttempts()), 0);
retryDelay = Const.toLongExpanded(resolve(meta.getRetryDelay()), 0L);
```
That the values such like `1e0`, `100k`, and resolved variables are now
handled correctly instead of silently falling back to `0`.
Also added the following unit test:
- `executeWithRetriesParsesExpandedRetryAttempts`
## Additional review suggestions
All three additional suggestions have also been addressed:
- Before starting a new attempt, `PipelineExecutor` now stops any
still-running child pipeline from the previous attempt and removes that
attempt’s log channel.
- The same cleanup is also performed in the `HopException` path within
`executePipelineAttempt()`.
- Replaced the hard-coded interruption message with the i18n key
`PipelineExecutor.Log.RetryInterrupted` and added the corresponding entry to
`messages_en_US.properties`.
- Updated `pipeline-executor.adoc` to document:
- **Retry attempts on failure**
- **Retry delay (milliseconds)**, including the default no-retry behavior
- **Stop after (milliseconds)** applies independently to each execution
attempt
- That the retry delay occurs between attempts and is outside the
per-attempt timeout
--
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]
