janhoy commented on a change in pull request #544:
URL: https://github.com/apache/solr/pull/544#discussion_r788243545
##########
File path: solr/core/src/java/org/apache/solr/update/UpdateLog.java
##########
@@ -372,28 +373,34 @@ public void init(UpdateHandler uhandler, SolrCore core) {
return;
}
lastDataDir = dataDir;
- tlogDir = new File(dataDir, TLOG_NAME);
- tlogDir.mkdirs();
- tlogFiles = getLogList(tlogDir);
+ tlogDir = Path.of(dataDir, TLOG_NAME);
+ try {
+ Files.createDirectories(tlogDir);
+ } catch (IOException e) {
+ throw new SolrException(ErrorCode.SERVER_ERROR, "Could not set up
tlogs", e);
+ }
+ tlogFiles = getLogList(tlogDir.toFile());
id = getLastLogId() + 1; // add 1 since we will create a new log for the
next update
if (debug) {
log.debug("UpdateHandler init: tlogDir={}, existing tlogs={}, next
id={}", tlogDir, Arrays.asList(tlogFiles), id);
}
- String[] oldBufferTlog = getBufferLogList(tlogDir);
- if (oldBufferTlog != null && oldBufferTlog.length != 0) {
- existOldBufferLog = true;
+ final String prefix = BUFFER_TLOG_NAME + '.';
+ try (Stream<Path> bufferedTLogs = Files.walk(tlogDir, 1)) {
+ existOldBufferLog = bufferedTLogs.anyMatch(path ->
path.getFileName().toString().startsWith(prefix));
+ } catch (IOException e) {
+ // TODO
Review comment:
TODO
##########
File path: solr/core/src/java/org/apache/solr/update/UpdateLog.java
##########
@@ -1328,25 +1325,27 @@ public void copyOverOldUpdates(long commitVersion,
TransactionLog oldTlog) {
protected void ensureBufferTlog() {
if (bufferTlog != null) return;
String newLogName = String.format(Locale.ROOT, LOG_FILENAME_PATTERN,
BUFFER_TLOG_NAME, System.nanoTime());
- bufferTlog = newTransactionLog(new File(tlogDir, newLogName),
globalStrings, false);
+ bufferTlog = newTransactionLog(tlogDir.resolve(newLogName), globalStrings,
false);
bufferTlog.isBuffer = true;
}
// Cleanup old buffer tlogs
protected void deleteBufferLogs() {
- String[] oldBufferTlog = getBufferLogList(tlogDir);
- if (oldBufferTlog != null && oldBufferTlog.length != 0) {
- for (String oldBufferLogName : oldBufferTlog) {
- deleteFile(new File(tlogDir, oldBufferLogName));
- }
+ try (Stream<Path> tlogs = Files.walk(tlogDir, 1)) {
+ final String prefix = BUFFER_TLOG_NAME + '.';
+ tlogs.filter(Files::isRegularFile)
+ .filter(path -> path.getFileName().toString().startsWith(prefix))
+ .forEach(UpdateLog::deleteFile);
+ } catch (IOException e) {
+ log.warn("Could not clean up buffered transaction logs in {}", tlogDir,
e);
Review comment:
Nice that you also add more informative exception messages
##########
File path: solr/core/src/java/org/apache/solr/update/UpdateLog.java
##########
@@ -2133,18 +2132,18 @@ public static AddUpdateCommand
convertTlogEntryToAddUpdateCommand(SolrQueryReque
new SolrNamedThreadFactory("recoveryExecutor"));
- public static void deleteFile(File file) {
+ public static void deleteFile(Path file) {
boolean success = false;
try {
- Files.deleteIfExists(file.toPath());
+ Files.deleteIfExists(file);
success = true;
} catch (Exception e) {
log.error("Error deleting file: {}", file, e);
}
if (!success) {
try {
- file.deleteOnExit();
+ file.toFile().deleteOnExit();
Review comment:
Just looks a bit weird with `file.toFile()`, but probably ok. Have not
checked if there is a deleteOnExit API on Files..
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
