jnioche commented on PR #9182: URL: https://github.com/apache/storm/pull/9182#issuecomment-6336525952
**Earlier points, now fixed** **Log list:** /listLogs and getAllLogsForRootDir now leave out links at the topology, port and file level. This also fixes a bug in the topology-plus-port branch: it used getCanonicalFile(), which made it return an empty list under a linked log root. **Late link:** a file that becomes a link between the check and the open now returns 404 rather than a 500, because openForReading returns null. Page, search, download, dump download and deep search are all covered and tested. **Assumptions documented:** the class javadoc now states why the race is limited to the last path element (the topology dir stays owned by the supervisor) and that hard links rely on fs.protected_hardlinks. **One gzip length helper:** the daemon page now also takes its length from the opened file. **Dump name check:** I’d called the getNameCount() != 1 check redundant; the new comment shows it isn’t, because an empty port such as host::1111 would otherwise reach another port directory. **Remaining nits, none blocking** **Deleted files still give a 500.** If the log cleaner or log rotation removes a file between the check and the open, NoSuchFileException is rethrown. This was already the case before the PR, but it’s a more likely race than a link swap, and returning null for it in openForReading would give a 404. **Dead code:** LogviewerResponseBuilder.buildDownloadFile(String, File, Meter) and ServerUtils.zipFileSize are no longer used anywhere. Remove the first; ServerUtils is public, so deprecating zipFileSize is enough. **Download size metric:** LogFileDownloader still records the size with file.toFile().length() before opening the file. channel.size() would match the “length from the opened file” approach used elsewhere. **Port validation:** rejecting a non-numeric portStr in the dump endpoints would be more robust than the name-count check. **Scanned-file count:** findNMatches counts a file that turned into a link as scanned. -- 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]
