bamaer commented on PR #8674:
URL: https://github.com/apache/hop/pull/8674#issuecomment-5932614978
**Blockers**
- **Files can be left unclosed at the end of the run.** Both
`flushOpenFiles` implementations close a stream only if it's dirty
(`TextFileOutputData.java:214-216` and `:406-408`). Before this PR the interval
flush never ran, so open files always stayed dirty. Now `FileStream.flush()`
clears the flag, and `dispose()` → `close()` → `flushOpenFiles(true)` skips
every stream that hasn't been written to since the last interval flush.
- **Single file:** this happens whenever the interval flush runs after the
last row, which with slow input (the case #2406 is about) is almost every run.
- **Filename taken from a field:** every file not written to since the
last interval flush stays open.
- **Result:**
- gzip without a trailer, zip without a central directory
- S3, Azure and HDFS objects never committed
- open handles on local files
`TextFileOutputFlushTest` never ends the stream, so it doesn't catch this.
Suggested fix: in both implementations, flush when dirty, but close
whenever `closeAfterFlush && isOpen()`, whatever the dirty flag says. Add a
test where the interval flush runs after the last row, the stream then ends,
and the file is closed and readable (for example a gzip file that decompresses).
**Should fix**
- **The new flush can't be switched off.** `getFlushInterval()`
(`TextFileOutput.java:298`) maps `0`, negative values and non-numbers to 5000.
The interval flush never ran on `main`, so every existing Text File Output,
including high-volume and remote targets, now flushes every 5 seconds with no
way to opt out. Existing `hop-config.json` files store `0` for this variable.
Suggested fix: let a negative value (for example `-1`) disable the
interval flush, and document it in `Const`, `variables.adoc` and
`textfileoutput.adoc`.
- **Every row can scan all open files when the filename comes from a
field.** `markCurrentFileDirty()` (`TextFileOutput.java:831`) is called for
each row. When rows alternate between files, `getLastStream()` isn't the
current file, so it walks every open stream (up to
`HOP_FILE_OUTPUT_MAX_STREAM_COUNT`, 1024 by default). `initFileStreamWriter`
already marks the stream dirty on that path.
Suggested fix: keep a reference to the current `FileStream` when
`data.writer` is assigned, and mark that one dirty directly.
--
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]
