goutamadwant commented on code in PR #12299:
URL: https://github.com/apache/seatunnel/pull/12299#discussion_r4000362850


##########
seatunnel-common/src/main/java/org/apache/seatunnel/common/utils/FileUtils.java:
##########
@@ -75,6 +79,63 @@ public static String readFileToStr(Path path) {
         }
     }
 
+    /**
+     * Reads a file, keeping at most {@code maxBytes} bytes from the end of it.
+     *
+     * <p>Reading a file whole materialises it twice on the heap, once as a 
byte array and once as a
+     * string. For files that can grow without bound - engine log files being 
the case this was
+     * written for - that turns a single read into a node-wide memory problem. 
When the file is
+     * larger than the limit its tail is returned instead, the tail being the 
part that matters when
+     * diagnosing a failure.
+     *
+     * <p>The tail starts at the first line break after the cut point, so it 
never begins with half
+     * a line and never splits a multi-byte UTF-8 character. Content is 
decoded as UTF-8 rather than
+     * with the platform default charset used by {@link #readFileToStr(Path)}, 
because aligning on
+     * character boundaries is only meaningful against a known encoding.
+     *
+     * @param path file to read
+     * @param maxBytes maximum number of bytes to keep from the end; a value 
<= 0 means unlimited
+     * @return the whole file, or its tail when the file is larger than {@code 
maxBytes}
+     */
+    public static String readFileTailToStr(Path path, long maxBytes) {
+        if (maxBytes <= 0) {
+            return readFileToStr(path);
+        }
+        try (SeekableByteChannel channel = Files.newByteChannel(path, 
StandardOpenOption.READ)) {
+            long size = channel.size();
+            if (size <= maxBytes) {

Review Comment:
   Please clamp the effective limit before this `size <= maxBytes` branch. I 
reproduced this with a sparse 3 GiB file and a 4 GiB limit, which is the value 
produced by `log-response-max-size-mb: 4096`: this branch calls 
`readFileToStr`, and `Files.readAllBytes` throws `OutOfMemoryError: Required 
array size too large`. Both REST paths therefore still allow a supported 
positive setting to trigger the node-wide failure this PR is intended to 
prevent. Either reject values above the maximum representable response or 
compare `size` against the clamped effective byte limit and use the tail path.



##########
seatunnel-common/src/main/java/org/apache/seatunnel/common/utils/FileUtils.java:
##########
@@ -75,6 +79,63 @@ public static String readFileToStr(Path path) {
         }
     }
 
+    /**
+     * Reads a file, keeping at most {@code maxBytes} bytes from the end of it.
+     *
+     * <p>Reading a file whole materialises it twice on the heap, once as a 
byte array and once as a
+     * string. For files that can grow without bound - engine log files being 
the case this was
+     * written for - that turns a single read into a node-wide memory problem. 
When the file is
+     * larger than the limit its tail is returned instead, the tail being the 
part that matters when
+     * diagnosing a failure.
+     *
+     * <p>The tail starts at the first line break after the cut point, so it 
never begins with half
+     * a line and never splits a multi-byte UTF-8 character. Content is 
decoded as UTF-8 rather than
+     * with the platform default charset used by {@link #readFileToStr(Path)}, 
because aligning on
+     * character boundaries is only meaningful against a known encoding.
+     *
+     * @param path file to read
+     * @param maxBytes maximum number of bytes to keep from the end; a value 
<= 0 means unlimited
+     * @return the whole file, or its tail when the file is larger than {@code 
maxBytes}
+     */
+    public static String readFileTailToStr(Path path, long maxBytes) {
+        if (maxBytes <= 0) {
+            return readFileToStr(path);
+        }
+        try (SeekableByteChannel channel = Files.newByteChannel(path, 
StandardOpenOption.READ)) {
+            long size = channel.size();
+            if (size <= maxBytes) {
+                return readFileToStr(path);
+            }
+            // A String cannot hold more than Integer.MAX_VALUE chars anyway, 
so a limit above that
+            // can never take effect and clamping keeps the cast below safe.
+            int keep = (int) Math.min(maxBytes, Integer.MAX_VALUE - 8);
+            ByteBuffer buffer = ByteBuffer.allocate(keep);
+            channel.position(size - keep);
+            while (buffer.hasRemaining() && channel.read(buffer) > 0) {
+                // Keep reading until the requested tail is filled or the file 
ends.
+            }
+            return new String(
+                    tailFromLineStart(buffer.array(), buffer.position()), 
StandardCharsets.UTF_8);
+        } catch (IOException e) {
+            throw CommonError.fileOperationFailed("SeaTunnel", "read", 
path.toString(), e);
+        }
+    }
+
+    private static byte[] tailFromLineStart(byte[] bytes, int length) {
+        for (int i = 0; i < length; i++) {
+            if (bytes[i] == '\n') {
+                return Arrays.copyOfRange(bytes, i + 1, length);

Review Comment:
   Please handle a terminator-only boundary as the oversized-single-line case. 
I reproduced `readFileTailToStr` on `abcdefghijklmnopqrstuvwxyz\n` with 
`maxBytes = 10`; the retained window contains nine data bytes plus the final 
newline, so this returns an empty string. Log appenders normally terminate each 
event, so one oversized event at the end of a file makes the REST endpoint 
return no log content. If the first newline is the last retained byte, keep a 
UTF-8-safe partial tail instead and add this case to `FileUtilsTest`.



-- 
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]

Reply via email to