SEZ9 opened a new pull request, #12299:
URL: https://github.com/apache/seatunnel/pull/12299
### Purpose of this pull request
Closes #12295.
The REST log-content endpoints read the whole file into a `String` before
writing the response:
```java
// LogBaseServlet#prepareLogResponse
String logContent = FileUtils.readFileToStr(new
File(canonicalFilePath).toPath());
write(resp, logContent);
```
```java
// FileUtils#readFileToStr
byte[] bytes = Files.readAllBytes(path);
return new String(bytes);
```
That materialises the file twice on the heap, once as a byte array and once
as a string, with no upper bound. A streaming job's log file grows without
limit, so a single `GET /logs/<name>` on a job that has been running for weeks
was enough to push the node into a long GC pause or an `OutOfMemoryError` —
taking down the engine node rather than just failing the request.
`RestHttpGetCommandProcessor` carries an equivalent copy for REST v1 and had
the same problem.
This adds `seatunnel.engine.http.log-response-max-size-mb` and reads at most
that much from the **end** of the file, the tail being the part that matters
when diagnosing a failure.
The existing path-traversal guard in `prepareLogResponse` is untouched; this
is purely about response size.
### Does this PR introduce _any_ user-facing change?
Yes.
**New option `seatunnel.engine.http.log-response-max-size-mb`, default
`64`.** A log file larger than the limit is truncated to its last 64 MB instead
of being returned whole. Setting the option to `0` or a negative value restores
the previous unlimited behaviour, matching how `upload-max-file-size-mb` treats
non-positive values.
On the default: 64 MB of text is already far past what a browser or a human
consumes, and the transient cost of serving it is roughly 64 MB for the byte
array plus up to 128 MB for the string. Picking a much larger default would
keep a single request able to exhaust a typical node heap, which is the thing
being fixed. Anyone who genuinely needs whole multi-gigabyte files can set `0`
and accept the heap cost explicitly.
Truncation is aligned to the first line break after the cut point, so a
response never begins mid-line, and never begins in the middle of a multi-byte
UTF-8 character — slicing at an arbitrary byte offset would otherwise make a
CJK log start with a replacement character. Where a single line is longer than
the whole limit there is no line boundary to align to, so only the leading
UTF-8 continuation bytes are dropped.
Content in the new path is decoded as UTF-8 explicitly, whereas
`readFileToStr` uses the platform default charset. Reasoning about character
boundaries is only meaningful against a known encoding, and log4j2 writes
UTF-8. I deliberately did not change `readFileToStr` itself — that would touch
every caller and belongs in its own change.
Docs updated in `docs/en/engines/zeta/rest-api-v2.md` and
`docs/zh/engines/zeta/rest-api-v2.md`: the code-defaults list, the config
example, and a note under both the all-node and single-node log endpoints.
### How was this patch tested?
Unit tests in `FileUtilsTest`, covering `readFileTailToStr`:
- a file below the limit, and a file exactly at the limit, are returned
whole;
- a non-positive limit means unlimited;
- an empty file returns `""`;
- above the limit the result is a suffix of the file, smaller than the
limit, starting at a line boundary and reaching the end of the file;
- **UTF-8 safety**: a file of three-byte CJK characters is read with every
limit from 100 to 400 bytes, so some cut points necessarily land inside a
character. Each result is asserted to contain no `U+FFFD`, to be a suffix of
the file, and to start at a line boundary;
- a single line longer than the limit, where there is no boundary to align
to, still decodes without a replacement character.
The new config option is also asserted in `YamlSeaTunnelConfigParserTest`
against a non-default value in the test `seatunnel.yaml`. That is not ceremony:
an `http` option without a matching branch in
`YamlSeaTunnelDomConfigProcessor.parseHttpConfig` is silently dropped with an
`"Unrecognized element"` warning, so without that assertion the option could
ship inert and look fine.
I do not have a JDK/Maven in my working environment, so I have not run the
suite locally — CI is the first execution of these changes.
One thing I left as-is for reviewers to weigh in on: `LogBaseServlet` and
`RestHttpGetCommandProcessor` each resolve the limit separately, because the v1
and v2 code paths sit in different class hierarchies and already duplicate the
whole of `prepareLogResponse`. Unifying them is a larger refactor than this fix
warrants, but I am happy to do it here if you would rather.
### Check list
* [x] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md)
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
* [x] If you are contributing the connector code, please check that the
following files are updated:
--
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]
