ArvinDevel commented on code in PR #3194:
URL: https://github.com/apache/bookkeeper/pull/3194#discussion_r929946654


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java:
##########
@@ -153,23 +153,32 @@ private JournalChannel(File journalDirectory, long logId,
                            long position, boolean fRemoveFromPageCache,
                            int formatVersionToWrite, 
Journal.BufferedChannelBuilder bcBuilder,
                            ServerConfiguration conf,
-                           FileChannelProvider provider) throws IOException {
+                           FileChannelProvider provider, Long toReplaceLogId) 
throws IOException {
         this.journalAlignSize = journalAlignSize;
         this.zeros = ByteBuffer.allocate(journalAlignSize);
         this.preAllocSize = preAllocSize - preAllocSize % journalAlignSize;
         this.fRemoveFromPageCache = fRemoveFromPageCache;
         this.configuration = conf;
 
+        boolean reuseFile = false;
         File fn = new File(journalDirectory, Long.toHexString(logId) + ".txn");
+        if (toReplaceLogId != null && logId != toReplaceLogId && 
provider.supportReuseFile()) {
+            File toReplaceFile = new File(journalDirectory, 
Long.toHexString(toReplaceLogId) + ".txn");
+            if (toReplaceFile.exists()) {
+                renameJournalFile(toReplaceFile, fn);
+                provider.notifyRename(toReplaceFile, fn);
+                reuseFile = true;
+            }
+        }
         channel = provider.open(fn, configuration);
 
         if (formatVersionToWrite < V4) {
             throw new IOException("Invalid journal format to write : version = 
" + formatVersionToWrite);
         }
 
         LOG.info("Opening journal {}", fn);
-        if (!channel.fileExists(fn)) { // new file, write version
-            if (!fn.createNewFile()) {
+        if (!channel.fileExists(fn) || reuseFile) { // new file, write version
+            if (!reuseFile && !fn.createNewFile()) {

Review Comment:
   why mix the logic with the old one?
   when the reuseFile is true, we have done `renameJournalFile(toReplaceFile, 
fn)`, so we expect the `channel.fileExists(fn)`, if not then throw exception, 
is this alert what you want to ensure? 
   If so I suggest to separate it and put more clear information to log, and 
please also add necessary comment. 



-- 
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]

Reply via email to