Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2754#discussion_r209094640
--- Diff:
storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogSearchHandler.java
---
@@ -388,63 +414,83 @@ private Integer tryParseIntParam(String paramName,
String value) throws InvalidR
}
}
+ /**
+ * Find the first N matches of target string in files.
+ * @param logs all candidate log files to search
+ * @param numMatches number of matches expected
+ * @param fileOffset
Unclear metrics
+ * @param startByteOffset number of byte to be ignored in each log file
+ * @param targetStr searched string
+ * @return all matched results
+ */
@VisibleForTesting
- Matched findNMatches(List<File> logs, int numMatches, int fileOffset,
int offset, String search) {
+ Matched findNMatches(List<File> logs, int numMatches, int fileOffset,
int startByteOffset, String targetStr) {
logs = drop(logs, fileOffset);
+ LOG.debug("{} files to scan", logs.size());
List<Map<String, Object>> matches = new ArrayList<>();
int matchCount = 0;
+ int scannedFiles = 0;
+ //TODO: Unnecessarily convoluted loop that should be optimized
while (true) {
if (logs.isEmpty()) {
break;
}
File firstLog = logs.get(0);
- Map<String, Object> theseMatches;
+ Map<String, Object> matchInLog;
try {
LOG.debug("Looking through {}", firstLog);
- theseMatches = substringSearch(firstLog, search,
numMatches - matchCount, offset);
+ matchInLog = substringSearch(firstLog, targetStr,
numMatches - matchCount, startByteOffset);
+ scannedFiles++;
} catch (InvalidRequestException e) {
LOG.error("Can't search past end of file.", e);
- theseMatches = new HashMap<>();
+ matchInLog = new HashMap<>();
}
String fileName =
WorkerLogs.getTopologyPortWorkerLog(firstLog);
+ //This section simply put the formatted log filename and
corresponding port in the matching.
final List<Map<String, Object>> newMatches = new
ArrayList<>(matches);
- Map<String, Object> currentFileMatch = new
HashMap<>(theseMatches);
+ Map<String, Object> currentFileMatch = new
HashMap<>(matchInLog);
currentFileMatch.put("fileName", fileName);
Path firstLogAbsPath;
try {
firstLogAbsPath = firstLog.getCanonicalFile().toPath();
} catch (IOException e) {
throw new RuntimeException(e);
}
+ //Why do we need to start from scratch to retrieve just the
port here?
--- End diff --
Oh I was complaining that the implementation doesn't look too good. Both
port and filename are included in the absolute path. We could just optimize the
code here. It looks minor though.
---
