SEZ9 commented on PR #12299: URL: https://github.com/apache/seatunnel/pull/12299#issuecomment-5754501558
Thanks for the update. I can't confirm from the thread alone which of the earlier points have landed on `e513e6b130c4`, so a pointer to the relevant change for each would help a lot. Where things stand from my side: **Need a pointer to confirm:** - Truncation marker (F3/F4): the described approach (a shared `LogContentReader` prepending a one-line notice such as `[SeaTunnel] Log truncated: returning the last 67108864 bytes of 3435973836, ...`) is what I was after. Please point me at that code, and add an assertion in `LogContentReaderTest` that the notice is present when truncated and absent when the file fits. - Duplicated MB-to-bytes / `<= 0` sentinel (F8): if the conversion now lives in one place and `LogBaseServlet` / `LogService` delegate to it, this is closed; a quick pointer is enough. **Still open as far as I can tell:** - Charset consistency (F1): the within-limit path in `readFileTailToStr` should decode with the same explicit charset as the tail path rather than platform-default `readFileToStr`, so the same file is never read two different ways depending on its size. Please confirm or adjust. - Extra copies in `tailFromLineStart` (F5): the retained window should be decoded once from the byte buffer at the line-start offset without an intermediate copy. If you believe the current version already does this, please walk me through the allocations on the truncated path. - `incompatible-changes.md` (F2/F4): the default 64 MB tail changes what existing deployments get back from the v1 and v2 log endpoints. Please add an entry describing the new default and the `<= 0` opt-out. - v1 REST docs (F6): `rest-api-v1.md` (en and zh) needs the same note and option reference as the v2 page, since those endpoints are bounded too. - Doc wording (F7): describe the truncated size as "the configured `log-response-max-size-mb` (64 MB by default)" instead of a hard-coded "last 64 MB", and drop the stray double backtick above the new note on the zh page. None of this is large; once the charset and docs items are in, I expect this to be ready. <!-- streview-comment:1198 --> -- 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]
