This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new ca1f72d GEODE-5212: Ensure that MergeLogs closes its InputStreams
(#2513)
ca1f72d is described below
commit ca1f72df06ea7987cdc13ed19b16ee55bafd21f7
Author: Jens Deppe <[email protected]>
AuthorDate: Thu Sep 27 08:30:17 2018 -0700
GEODE-5212: Ensure that MergeLogs closes its InputStreams (#2513)
- Other refactoring which converted a pair of linked structures (log
filename
and the associated InputStream) into a Map.
---
.../internal/cli/util/MergeLogsDUnitTest.java | 9 ++++
.../internal/logging/MergeLogFilesJUnitTest.java | 12 ++---
.../apache/geode/admin/internal/LogCollator.java | 10 ++--
.../org/apache/geode/internal/SystemAdmin.java | 56 ++++++++--------------
.../geode/internal/logging/MergeLogFiles.java | 56 ++++++++++------------
.../management/internal/cli/util/MergeLogs.java | 11 +++--
6 files changed, 72 insertions(+), 82 deletions(-)
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/util/MergeLogsDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/util/MergeLogsDUnitTest.java
index e6cb601..67b40d3 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/util/MergeLogsDUnitTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/util/MergeLogsDUnitTest.java
@@ -26,8 +26,10 @@ import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.List;
import java.util.Properties;
+import java.util.concurrent.TimeUnit;
import org.apache.commons.io.FileUtils;
+import org.awaitility.Awaitility;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -59,12 +61,19 @@ public class MergeLogsDUnitTest {
MemberVM server = lsRule.startServerVM(1, properties, locator.getPort());
MemberVM server2 = lsRule.startServerVM(2, properties, locator.getPort());
+ // Especially on Windows, wait for time to pass otherwise all log messages
may appear in the
+ // same millisecond.
locator.invoke(() -> LogService.getLogger().info(MESSAGE_1));
+ Awaitility.await().atLeast(1, TimeUnit.MILLISECONDS).until(() -> true);
server.invoke(() -> LogService.getLogger().info(MESSAGE_2));
+ Awaitility.await().atLeast(1, TimeUnit.MILLISECONDS).until(() -> true);
server2.invoke(() -> LogService.getLogger().info(MESSAGE_3));
+ Awaitility.await().atLeast(1, TimeUnit.MILLISECONDS).until(() -> true);
locator.invoke(() -> LogService.getLogger().info(MESSAGE_4));
+ Awaitility.await().atLeast(1, TimeUnit.MILLISECONDS).until(() -> true);
server.invoke(() -> LogService.getLogger().info(MESSAGE_5));
+ Awaitility.await().atLeast(1, TimeUnit.MILLISECONDS).until(() -> true);
server2.invoke(() -> LogService.getLogger().info(MESSAGE_6));
}
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/internal/logging/MergeLogFilesJUnitTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/internal/logging/MergeLogFilesJUnitTest.java
index 0228ef3..bf99ea5 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/internal/logging/MergeLogFilesJUnitTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/internal/logging/MergeLogFilesJUnitTest.java
@@ -28,8 +28,10 @@ import java.io.PrintWriter;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Random;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -81,20 +83,18 @@ public class MergeLogFilesJUnitTest {
}
// Merge the log files together
- InputStream[] streams = new InputStream[workers.size()];
- String[] names = new String[workers.size()];
+ Map<String, InputStream> logs = new HashMap<>();
for (int i = 0; i < workers.size(); i++) {
Worker worker = (Worker) workers.get(i);
- streams[i] = worker.getInputStream();
- names[i] = worker.getName();
+ logs.put(worker.getName(), worker.getInputStream());
}
StringWriter sw = new StringWriter();
PrintWriter pw = new PrintWriter(sw, true);
- MergeLogFiles.mergeLogFiles(streams, names, pw);
+ MergeLogFiles.mergeLogFiles(logs, pw);
// System.out.println(sw.toString());
- // Verfiy that the entries are sorted
+ // Verify that the entries are sorted
BufferedReader br = new BufferedReader(new StringReader(sw.toString()));
Pattern pattern = Pattern.compile("^Worker \\d+: .* VALUE: (\\d+)");
int lastValue = -1;
diff --git
a/geode-core/src/main/java/org/apache/geode/admin/internal/LogCollator.java
b/geode-core/src/main/java/org/apache/geode/admin/internal/LogCollator.java
index 4fbf629..f446919 100755
--- a/geode-core/src/main/java/org/apache/geode/admin/internal/LogCollator.java
+++ b/geode-core/src/main/java/org/apache/geode/admin/internal/LogCollator.java
@@ -20,7 +20,9 @@ import java.io.InputStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import org.apache.geode.internal.admin.ApplicationVM;
import org.apache.geode.internal.admin.GemFireVM;
@@ -54,18 +56,16 @@ public class LogCollator {
private String mergeLogs() {
// combine logs...
- InputStream[] logFiles = new InputStream[this.logTails.size()];
- String[] logFileNames = new String[logFiles.length];
+ Map<String, InputStream> logFiles = new HashMap<>();
for (int i = 0; i < this.logTails.size(); i++) {
Loglet loglet = (Loglet) this.logTails.get(i);
- logFiles[i] = new ByteArrayInputStream(loglet.tail.getBytes());
- logFileNames[i] = loglet.name;
+ logFiles.put(loglet.name, new
ByteArrayInputStream(loglet.tail.getBytes()));
}
// delegate to MergeLogFiles...
StringWriter writer = new StringWriter();
PrintWriter mergedLog = new PrintWriter(writer);
- if (!MergeLogFiles.mergeLogFiles(logFiles, logFileNames, mergedLog)) {
+ if (!MergeLogFiles.mergeLogFiles(logFiles, mergedLog)) {
return writer.toString();
} else {
return "";
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java
b/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java
index c6d3738..f9d35a9 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java
@@ -26,6 +26,7 @@ import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.PrintStream;
@@ -38,6 +39,7 @@ import java.net.SocketException;
import java.net.URL;
import java.net.URLDecoder;
import java.net.UnknownHostException;
+import java.nio.file.Paths;
import java.text.DateFormat;
import java.text.ParseException;
import java.util.ArrayList;
@@ -55,8 +57,11 @@ import java.util.Set;
import java.util.UUID;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import java.util.stream.Collectors;
import java.util.zip.GZIPInputStream;
+import org.apache.commons.lang.StringUtils;
+
import org.apache.geode.GemFireException;
import org.apache.geode.GemFireIOException;
import org.apache.geode.InternalGemFireException;
@@ -783,9 +788,8 @@ public class SystemAdmin {
}
}
- public void mergeLogs(String outOption, List args) {
- FileInputStream[] input = new FileInputStream[args.size()];
- String[] inputNames = new String[args.size()]; // note we don't want any
extra names printed.
+ public void mergeLogs(String outOption, List<String> args) {
+ Map<String, InputStream> inputs = new HashMap<>();
PrintStream ps;
if (outOption != null) {
@@ -801,17 +805,20 @@ public class SystemAdmin {
}
PrintWriter mergedFile = new PrintWriter(ps, true);
- Iterator it = args.iterator();
- int idx = 0;
+ List<String> normalizedFiles =
+ args.stream().map(f ->
Paths.get(f).toAbsolutePath().toString()).collect(
+ Collectors.toList());
+ int prefixLength =
+ StringUtils.getCommonPrefix(normalizedFiles.toArray(new String[]
{})).length();
+
if (!quiet) {
ps.println(LocalizedStrings.SystemAdmin_MERGING_THE_FOLLOWING_LOG_FILES.toLocalizedString());
}
- while (it.hasNext()) {
- String fileName = (String) it.next();
+
+ for (String fileName : normalizedFiles) {
try {
- input[idx] = new FileInputStream(fileName);
- inputNames[idx] = (new File(fileName)).getAbsolutePath();
- idx++;
+ String shortName = fileName.substring(prefixLength);
+ inputs.put(shortName, new FileInputStream(fileName));
} catch (FileNotFoundException ex) {
throw new GemFireIOException(
LocalizedStrings.SystemAdmin_COULD_NOT_OPEN_TO_0_FOR_READING_BECAUSE_1
@@ -822,31 +829,7 @@ public class SystemAdmin {
}
}
- if (idx > 0) {
- // strip off any common filename prefix
- boolean strip = true;
- do {
- if (inputNames[0].length() == 0) {
- break;
- }
- if (inputNames[0].indexOf('/') == -1 && inputNames[0].indexOf('\\') ==
-1) {
- // no more directories to strip off
- break;
- }
- char c = inputNames[0].charAt(0);
- for (int i = 1; i < idx; i++) {
- if (inputNames[i].charAt(0) != c) {
- strip = false;
- break;
- }
- }
- for (int i = 0; i < idx; i++) {
- inputNames[i] = inputNames[i].substring(1);
- }
- } while (strip);
- }
-
- if (MergeLogFiles.mergeLogFiles(input, inputNames, mergedFile)) {
+ if (MergeLogFiles.mergeLogFiles(inputs, mergedFile)) {
throw new GemFireIOException(
LocalizedStrings.SystemAdmin_TROUBLE_MERGING_LOG_FILES.toLocalizedString());
}
@@ -857,7 +840,8 @@ public class SystemAdmin {
if (!quiet) {
System.out
.println(LocalizedStrings.SystemAdmin_COMPLETED_MERGE_OF_0_LOGS_TO_1.toLocalizedString(
- new Object[] {Integer.valueOf(idx), ((outOption != null) ?
outOption : "stdout")}));
+ new Object[] {Integer.valueOf(args.size()),
+ ((outOption != null) ? outOption : "stdout")}));
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/logging/MergeLogFiles.java
b/geode-core/src/main/java/org/apache/geode/internal/logging/MergeLogFiles.java
index 8918a1d..5f2cdc7 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/logging/MergeLogFiles.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/logging/MergeLogFiles.java
@@ -27,9 +27,11 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.BlockingQueue;
@@ -102,7 +104,6 @@ public class MergeLogFiles {
* <code>PrinWriter</code>.
*
* @param logFiles The log files to be merged
- * @param logFileNames The names of the log files to be printed in the
merged log
* @param mergedFile Where the merged logs are printed to
*
* @return Whether or not problems occurred while merging the log files.
@@ -110,9 +111,8 @@ public class MergeLogFiles {
* @throws IllegalArgumentException If the length of <code>logFiles</code>
is not the same as the
* length of <code>logFileNames</code>
*/
- public static boolean mergeLogFiles(InputStream[] logFiles, String[]
logFileNames,
- PrintWriter mergedFile) {
- return mergeLogFiles(logFiles, logFileNames, mergedFile, false, false,
false, new LinkedList());
+ public static boolean mergeLogFiles(Map<String, InputStream> logFiles,
PrintWriter mergedFile) {
+ return mergeLogFiles(logFiles, mergedFile, false, false, false, new
LinkedList());
}
/**
@@ -120,7 +120,6 @@ public class MergeLogFiles {
* <code>PrinWriter</code>.
*
* @param logFiles The log files to be merged
- * @param logFileNames The names of the log files to be printed in the
merged log
* @param mergedFile Where the merged logs are printed to
* @param tabOut Whether to align non-timestamped lines with timestamped
lines
* @param suppressBlanks Whether to omit blank lines
@@ -130,10 +129,10 @@ public class MergeLogFiles {
* @throws IllegalArgumentException If the length of <code>logFiles</code>
is not the same as the
* length of <code>logFileNames</code>
*/
- public static boolean mergeLogFiles(InputStream[] logFiles, String[]
logFileNames,
+ public static boolean mergeLogFiles(Map<String, InputStream> logFiles,
PrintWriter mergedFile, boolean tabOut, boolean suppressBlanks, boolean
multithreaded,
List<String> patterns) {
- return Sorter.mergeLogFiles(logFiles, logFileNames, mergedFile, tabOut,
suppressBlanks,
+ return Sorter.mergeLogFiles(logFiles, mergedFile, tabOut, suppressBlanks,
multithreaded, patterns);
}
@@ -307,17 +306,16 @@ public class MergeLogFiles {
nickNames = findPIDs(files, mergedFile);
}
- InputStream[] logFiles = new InputStream[files.size()];
- String[] logFileNames = new String[files.size()];
+ Map<String, InputStream> logFiles = new HashMap<>();
for (int i = 0; i < files.size(); i++) {
File file = (File) files.get(i);
- logFiles[i] = new FileInputStream(file);
+ String logFileName;
if (findPIDs && (nickNames.get(i) != null)) {
if (file.getCanonicalPath().toLowerCase().endsWith("gz")) {
- logFileNames[i] = (String) nickNames.get(i) + ".gz";
+ logFileName = nickNames.get(i) + ".gz";
} else {
- logFileNames[i] = (String) nickNames.get(i);
+ logFileName = (String) nickNames.get(i);
}
} else {
StringBuffer sb = new StringBuffer();
@@ -334,11 +332,12 @@ public class MergeLogFiles {
}
sb.append(file.getName());
- logFileNames[i] = sb.toString();
+ logFileName = sb.toString();
}
+ logFiles.put(logFileName, new FileInputStream(file));
}
- mergeLogFiles(logFiles, logFileNames, mergedFile, tabOut, suppressBlanks,
multithreaded,
+ mergeLogFiles(logFiles, mergedFile, tabOut, suppressBlanks, multithreaded,
patterns);
ExitCode.NORMAL.doSystemExit();
@@ -650,7 +649,8 @@ public class MergeLogFiles {
/**
* Creates a new <code>Reader</code> that reads from the given log file
with the given name.
- * Invoking this constructor will start this reader thread.
+ * Invoking this constructor will start this reader thread. The
InputStream is closed at the
+ * end of processing.
*
* @param patterns TODO
*
@@ -729,6 +729,12 @@ public class MergeLogFiles {
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
+ } finally {
+ try {
+ logFile.close();
+ } catch (IOException e) {
+ e.printStackTrace(System.err);
+ }
}
}
@@ -824,7 +830,6 @@ public class MergeLogFiles {
* <code>PrintWriter</code>.
*
* @param logFiles The log files to be merged
- * @param logFileNames The names of the log files to be printed in the
merged log
* @param mergedFile Where the merged logs are printed to
* @param tabOut Whether to align non-timestamped lines with others
* @param suppressBlanks Whether to suppress output of blank lines
@@ -834,16 +839,9 @@ public class MergeLogFiles {
* @throws IllegalArgumentException If the length of <code>logFiles</code>
is not the same as
* the length of <code>logFileNames</code>
*/
- public static boolean mergeLogFiles(InputStream[] logFiles, String[]
logFileNames,
+ public static boolean mergeLogFiles(Map<String, InputStream> logFiles,
PrintWriter mergedFile, boolean tabOut, boolean suppressBlanks,
boolean multithreaded,
List<String> patterns) {
- if (logFiles.length != logFileNames.length) {
- throw new IllegalArgumentException(
-
LocalizedStrings.MergeLogFiles_NUMBER_OF_LOG_FILES_0_IS_NOT_THE_SAME_AS_THE_NUMBER_OF_LOG_FILE_NAMES_1
- .toLocalizedString(new Object[]
{Integer.valueOf(logFiles.length),
- Integer.valueOf(logFileNames.length)}));
- }
-
List<Pattern> compiledPatterns = new LinkedList<Pattern>();
for (String pattern : patterns) {
compiledPatterns.add(Pattern.compile(pattern,
Pattern.CASE_INSENSITIVE));
@@ -852,13 +850,13 @@ public class MergeLogFiles {
// First start the Reader threads
ReaderGroup group =
new
ReaderGroup(LocalizedStrings.MergeLogFiles_READER_THREADS.toLocalizedString());
- Collection readers = new ArrayList(logFiles.length);
- for (int i = 0; i < logFiles.length; i++) {
+ Collection readers = new ArrayList(logFiles.size());
+ for (Map.Entry<String, InputStream> e : logFiles.entrySet()) {
if (multithreaded) {
- readers.add(new ThreadedReader(logFiles[i], logFileNames[i], group,
tabOut,
+ readers.add(new ThreadedReader(e.getValue(), e.getKey(), group,
tabOut,
suppressBlanks, compiledPatterns));
} else {
- readers.add(new NonThreadedReader(logFiles[i], logFileNames[i],
group, tabOut,
+ readers.add(new NonThreadedReader(e.getValue(), e.getKey(), group,
tabOut,
suppressBlanks, compiledPatterns));
}
}
@@ -870,7 +868,6 @@ public class MergeLogFiles {
Set sorted = sortReaders(readers);
while (!readers.isEmpty()) {
-
Reader oldest = null;
Iterator sortedIt = sorted.iterator();
if (!sortedIt.hasNext()) {
@@ -886,7 +883,6 @@ public class MergeLogFiles {
nextReaderTimestamp = nextInLine.peek().getTimestamp();
}
-
// if we've switched to a different reader, emit a blank line
// for readability
if (oldest != lastOldest) {
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/MergeLogs.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/MergeLogs.java
index 496d6ed..cc1d838 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/MergeLogs.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/MergeLogs.java
@@ -27,7 +27,9 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import org.apache.commons.io.FileUtils;
import org.apache.logging.log4j.Logger;
@@ -101,12 +103,11 @@ public class MergeLogs {
static File mergeLogFile(String dirName) throws Exception {
Path dir = Paths.get(dirName);
List<File> logsToMerge = findLogFilesToMerge(dir.toFile());
- InputStream[] logFiles = new FileInputStream[logsToMerge.size()];
- String[] logFileNames = new String[logFiles.length];
+ Map<String, InputStream> logFiles = new HashMap<>();
for (int i = 0; i < logsToMerge.size(); i++) {
try {
- logFiles[i] = new FileInputStream(logsToMerge.get(i));
- logFileNames[i] =
dir.relativize(logsToMerge.get(i).toPath()).toString();
+ logFiles.put(dir.relativize(logsToMerge.get(i).toPath()).toString(),
+ new FileInputStream(logsToMerge.get(i)));
} catch (FileNotFoundException e) {
throw new Exception(
logsToMerge.get(i) + " " +
CliStrings.EXPORT_LOGS__MSG__FILE_DOES_NOT_EXIST);
@@ -120,7 +121,7 @@ public class MergeLogs {
+ new SimpleDateFormat("yyyy-MM-dd_HH-mm-ss").format(new
java.util.Date()) + ".log";
mergedLogFile = new File(mergeLog);
mergedLog = new PrintWriter(mergedLogFile);
- MergeLogFiles.mergeLogFiles(logFiles, logFileNames, mergedLog);
+ MergeLogFiles.mergeLogFiles(logFiles, mergedLog);
} catch (FileNotFoundException e) {
throw new Exception(
"FileNotFoundException in creating PrintWriter in MergeLogFiles" +
e.getMessage());
