-
Notifications
You must be signed in to change notification settings - Fork 5k
[Feature-17787] Support config max task log size for per task #17935
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
d0309b3
ea1232f
dc208f0
99b6463
ded47e8
42a3556
83f774c
852205d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ public class RemoteLogClient { | |
| * @return Returns the log content in byte array format. | ||
| */ | ||
| public byte[] getWholeLog(TaskInstance taskInstance) { | ||
| return LogUtils.getFileContentBytesFromRemote(taskInstance.getLogPath()); | ||
| return LogUtils.getFileContentBytesWithRollingLogs(taskInstance.getLogPath()); | ||
| } | ||
|
Comment on lines
35
to
37
|
||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,7 +29,10 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.nio.charset.StandardCharsets; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.nio.file.Files; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.nio.file.Paths; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Arrays; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.regex.Pattern; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.stream.Collectors; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.stream.Stream; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -79,11 +82,21 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int limit) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| File file = new File(filePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (file.exists() && file.isFile()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try (Stream<String> stream = Files.lines(Paths.get(filePath))) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return stream.skip(skipLine).limit(limit).collect(Collectors.toList()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (IOException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.error("read file error", e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new RuntimeException(String.format("Read file: %s error", filePath), e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.info("readPartFileContentFromLocal Reading log file"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if there are rolling log files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<File> logFiles = getRollingLogFiles(filePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (logFiles.size() > 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle rolling log files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return readFromRollingLogFiles(logFiles, skipLine, limit); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle single log file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try (Stream<String> stream = Files.lines(Paths.get(filePath))) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return stream.skip(skipLine).limit(limit).collect(Collectors.toList()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (IOException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.error("read file error", e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new RuntimeException(String.format("Read file: %s error", filePath), e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new RuntimeException("The file path: " + filePath + " not exists"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -169,4 +182,154 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return loggerContext.getProperty("log.base.ctx"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Get all rolling log files for a given base file path. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Returns a sorted list containing the base file and its rolled versions (e.g., .1, .2, etc.) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * ordered from newest to oldest (base file first, then .1, .2, etc.) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static List<File> getRollingLogFiles(String basePath) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<File> allFiles = new ArrayList<>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| File baseFile = new File(basePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| File parentDir = baseFile.getParentFile(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String fileName = baseFile.getName(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add the base file if it exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (baseFile.exists()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allFiles.add(baseFile); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Look for rolling files with pattern: basePath.N | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (parentDir != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| File[] files = parentDir.listFiles((dir, name) -> name.startsWith(fileName + ".") && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Pattern.matches(Pattern.quote(fileName) + "\\.\\d+", name)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (files != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allFiles.addAll(Arrays.asList(files)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Sort all files in reverse order based on rolling number | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Base file (without number) is treated as having number 0, so it comes last | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // descending order (larger numbers first) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allFiles.sort((file1, file2) -> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int num1 = isRollingFile(file1) ? extractRollingNumber(file1) : 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int num2 = isRollingFile(file2) ? extractRollingNumber(file2) : 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Integer.compare(num2, num1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return allFiles; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+185
to
+221
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Extract the rolling number from a file name (e.g., from "xxx.log.3" extract 3) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static int extractRollingNumber(File file) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String fileName = file.getName(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int lastDotIndex = fileName.lastIndexOf('.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (lastDotIndex != -1 && lastDotIndex < fileName.length() - 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Integer.parseInt(fileName.substring(lastDotIndex + 1)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (NumberFormatException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Integer.MAX_VALUE; // Put invalid files at the end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Integer.MAX_VALUE; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Check if the file is a rolling file (has a number suffix like .1, .2, etc.) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static boolean isRollingFile(File file) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String fileName = file.getName(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if the filename matches the pattern of a rolling file (basename.number) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int lastDotIndex = fileName.lastIndexOf('.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (lastDotIndex != -1 && lastDotIndex < fileName.length() - 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String suffix = fileName.substring(lastDotIndex + 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return suffix.matches("\\d+"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Read lines from multiple rolling log files in order | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static List<String> readFromRollingLogFiles(List<File> logFiles, int skipLine, int limit) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> allLines = new ArrayList<>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Read all lines from all log files in order | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (File file : logFiles) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.info("Reading log file: {}", file.getAbsolutePath()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try (Stream<String> stream = Files.lines(file.toPath())) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> fileLines = stream.collect(Collectors.toList()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allLines.addAll(fileLines); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (IOException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.error("Error reading file: " + file.getAbsolutePath(), e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new RuntimeException(String.format("Read file: %s error", file.getAbsolutePath()), e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check warning on line 268 in dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/LogUtils.java
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply skip and limit with overflow protection | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int startIndex = Math.min(skipLine, allLines.size()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Prevent integer overflow when calculating end index | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int endIndex; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (limit > allLines.size() - startIndex) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| endIndex = allLines.size(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| endIndex = startIndex + limit; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return allLines.subList(startIndex, endIndex); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+258
to
+282
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> allLines = new ArrayList<>(); | |
| // Read all lines from all log files in order | |
| for (File file : logFiles) { | |
| log.info("Reading log file: {}", file.getAbsolutePath()); | |
| try (Stream<String> stream = Files.lines(file.toPath())) { | |
| List<String> fileLines = stream.collect(Collectors.toList()); | |
| allLines.addAll(fileLines); | |
| } catch (IOException e) { | |
| log.error("Error reading file: " + file.getAbsolutePath(), e); | |
| throw new RuntimeException(String.format("Read file: %s error", file.getAbsolutePath()), e); | |
| } | |
| } | |
| // Apply skip and limit with overflow protection | |
| int startIndex = Math.min(skipLine, allLines.size()); | |
| // Prevent integer overflow when calculating end index | |
| int endIndex; | |
| if (limit > allLines.size() - startIndex) { | |
| endIndex = allLines.size(); | |
| } else { | |
| endIndex = startIndex + limit; | |
| } | |
| return allLines.subList(startIndex, endIndex); | |
| List<String> result = new ArrayList<>(); | |
| int remainingToSkip = Math.max(0, skipLine); | |
| int remainingToTake = limit < 0 ? 0 : limit; | |
| // Stream lines from log files in order, applying skip and limit across files | |
| for (File file : logFiles) { | |
| if (remainingToTake == 0) { | |
| break; | |
| } | |
| log.info("Reading log file: {}", file.getAbsolutePath()); | |
| try (Stream<String> stream = Files.lines(file.toPath())) { | |
| java.util.Iterator<String> iterator = stream.iterator(); | |
| while (iterator.hasNext() && remainingToTake > 0) { | |
| String line = iterator.next(); | |
| if (remainingToSkip > 0) { | |
| remainingToSkip--; | |
| continue; | |
| } | |
| result.add(line); | |
| remainingToTake--; | |
| } | |
| } catch (IOException e) { | |
| log.error("Error reading file: " + file.getAbsolutePath(), e); | |
| throw new RuntimeException(String.format("Read file: %s error", file.getAbsolutePath()), e); | |
| } | |
| } | |
| return result; |
Copilot
AI
Mar 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getFileContentBytesWithRollingLogs throws if the base log file doesn't exist, and it doesn't attempt RemoteLogUtils.getRemoteLog even when remote logging is enabled. Since this method is now used by RemoteLogClient, it can break log download when logs must be fetched from remote storage. Consider adding a remote-aware variant (download base + rolled files when remote logging is enabled) or integrating remote fallback into this method.
| } else { | |
| } else { | |
| // Local base log file does not exist, try to fetch from remote storage | |
| log.info("Local log file: {} does not exist, try to get remote log.", filePath); | |
| try { | |
| byte[] remoteLogBytes = RemoteLogUtils.getRemoteLog(filePath); | |
| if (remoteLogBytes != null && remoteLogBytes.length > 0) { | |
| return remoteLogBytes; | |
| } | |
| log.warn("Remote log file: {} is empty or not found.", filePath); | |
| } catch (Exception ex) { | |
| log.warn("Failed to get remote log file: {}", filePath, ex); | |
| } |
Check warning on line 306 in dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/LogUtils.java
SonarQubeCloud / SonarCloud Code Analysis
Replace generic exceptions with specific library exceptions or a custom exception.
See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ0-FyREoxp4O0LdRY4f&open=AZ0-FyREoxp4O0LdRY4f&pullRequest=17935
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,167 @@ | ||||||||||||
| /* | ||||||||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||||||||
| * this work for additional information regarding copyright ownership. | ||||||||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||
| * (the "License"); you may not use this file except in compliance with | ||||||||||||
| * the License. You may obtain a copy of the License at | ||||||||||||
| * | ||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||
| * | ||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||
| * limitations under the License. | ||||||||||||
| */ | ||||||||||||
|
|
||||||||||||
| package org.apache.dolphinscheduler.common.utils; | ||||||||||||
|
|
||||||||||||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||||||||||||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||||||||||||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||||||||||||
|
|
||||||||||||
| import java.io.File; | ||||||||||||
| import java.net.URL; | ||||||||||||
| import java.util.List; | ||||||||||||
|
|
||||||||||||
| import org.junit.jupiter.api.AfterEach; | ||||||||||||
| import org.junit.jupiter.api.BeforeEach; | ||||||||||||
| import org.junit.jupiter.api.Test; | ||||||||||||
|
|
||||||||||||
| public class LogUtilsTest { | ||||||||||||
|
Check warning on line 32 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
|
||||||||||||
|
|
||||||||||||
| private String predefinedLogPath; | ||||||||||||
|
|
||||||||||||
| @BeforeEach | ||||||||||||
| public void setUp() { | ||||||||||||
|
Check warning on line 37 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
|
||||||||||||
| URL resourceUrl = getClass().getClassLoader().getResource("log/730.log"); | ||||||||||||
| if (resourceUrl != null) { | ||||||||||||
| predefinedLogPath = resourceUrl.getPath(); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+36
to
+42
|
||||||||||||
|
|
||||||||||||
| @AfterEach | ||||||||||||
| public void tearDown() { | ||||||||||||
|
Check failure on line 45 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
|
||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @Test | ||||||||||||
| public void testReadPartFileContentFromLocal_Success() { | ||||||||||||
|
Check warning on line 49 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
|
||||||||||||
| if (predefinedLogPath != null) { | ||||||||||||
| String path = predefinedLogPath.replace("%20", " "); | ||||||||||||
| List<String> result = LogUtils.readPartFileContentFromLocal(path, 1, 3); | ||||||||||||
|
|
||||||||||||
| assertNotNull(result); | ||||||||||||
| assertTrue(result.size() >= 0); | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+49
to
+56
|
||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @Test | ||||||||||||
| public void testReadPartFileContentFromLocal_SkipNoneLimitAll() { | ||||||||||||
|
Check warning on line 60 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
|
||||||||||||
| if (predefinedLogPath != null) { | ||||||||||||
| String path = predefinedLogPath.replace("%20", " "); | ||||||||||||
| List<String> result = LogUtils.readPartFileContentFromLocal(path, 0, 100); | ||||||||||||
|
|
||||||||||||
| assertNotNull(result); | ||||||||||||
| assertTrue(result.size() >= 0); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @Test | ||||||||||||
| public void testReadPartFileContentFromPredefinedRollingFiles() { | ||||||||||||
|
Check warning on line 71 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
|
||||||||||||
| if (predefinedLogPath != null) { | ||||||||||||
| String mainLogPath = predefinedLogPath.replace("%20", " "); | ||||||||||||
|
|
||||||||||||
| File mainLogFile = new File(mainLogPath); | ||||||||||||
| assertTrue(mainLogFile.exists(), "Main log file should exist"); | ||||||||||||
|
|
||||||||||||
| File rollingLogFile = new File(mainLogPath + ".1"); | ||||||||||||
| assertTrue(rollingLogFile.exists(), "Rolling log file should exist"); | ||||||||||||
|
|
||||||||||||
| List<String> result = LogUtils.readPartFileContentFromLocal(mainLogPath, 0, 50); | ||||||||||||
|
|
||||||||||||
| assertNotNull(result); | ||||||||||||
| assertTrue(result.size() > 0, "Should have some content from the log files"); | ||||||||||||
|
|
||||||||||||
| System.out.println("Number of lines read: " + result.size()); | ||||||||||||
| for (int i = 0; i < Math.min(5, result.size()); i++) { | ||||||||||||
| System.out.println("Line " + i + ": " + result.get(i)); | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+85
to
+89
|
||||||||||||
| System.out.println("Number of lines read: " + result.size()); | |
| for (int i = 0; i < Math.min(5, result.size()); i++) { | |
| System.out.println("Line " + i + ": " + result.get(i)); | |
| } |
Check warning on line 94 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
SonarQubeCloud / SonarCloud Code Analysis
Remove this 'public' modifier.
See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ0-FyT6oxp4O0LdRY4m&open=AZ0-FyT6oxp4O0LdRY4m&pullRequest=17935
Check warning on line 105 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
SonarQubeCloud / SonarCloud Code Analysis
Remove this 'public' modifier.
See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ0-FyT6oxp4O0LdRY4n&open=AZ0-FyT6oxp4O0LdRY4n&pullRequest=17935
Check warning on line 116 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
SonarQubeCloud / SonarCloud Code Analysis
Remove this 'public' modifier.
See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ0-FyT6oxp4O0LdRY4o&open=AZ0-FyT6oxp4O0LdRY4o&pullRequest=17935
Check warning on line 123 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
SonarQubeCloud / SonarCloud Code Analysis
Remove this 'public' modifier.
See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ0-FyT6oxp4O0LdRY4p&open=AZ0-FyT6oxp4O0LdRY4p&pullRequest=17935
Check warning on line 146 in dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/LogUtilsTest.java
SonarQubeCloud / SonarCloud Code Analysis
Remove this 'public' modifier.
See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ0-FyT6oxp4O0LdRY4q&open=AZ0-FyT6oxp4O0LdRY4q&pullRequest=17935
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the local-log success path, this code only includes rolling logs if the API server can directly see the log file path on its local filesystem. In typical deployments logs reside on the worker/master host and are returned via the RPC (response.getLogBytes()), so rolling segments will still be omitted. The more reliable fix is to include rolling log handling in the provider side (ILogService/LogServiceImpl) or enhance the RPC response to include rolled files rather than trying to re-read local disk here.