[GitHub] [rocketmq] imaffe edited a comment on issue #2019: Should we always call destory before removing a mapped file ?

[GitBox <gi...@apache.org>]

imaffe edited a comment on issue #2019:
URL: https://github.com/apache/rocketmq/issues/2019#issuecomment-632930718


   Sorry I didn't make it claer. Actually this is found in resetOffset():
   
   ```
       public boolean resetOffset(long offset) {
           MappedFile mappedFileLast = getLastMappedFile();
   
           if (mappedFileLast != null) {
               long lastOffset = mappedFileLast.getFileFromOffset() +
                   mappedFileLast.getWrotePosition();
               long diff = lastOffset - offset;
   
               final int maxDiff = this.mappedFileSize * 2;
               if (diff > maxDiff)
                   return false;
           }
   
           ListIterator<MappedFile> iterator = this.mappedFiles.listIterator();
   
           while (iterator.hasPrevious()) {
               mappedFileLast = iterator.previous();
               if (offset >= mappedFileLast.getFileFromOffset()) {
                   int where = (int) (offset % mappedFileLast.getFileSize());
                   mappedFileLast.setFlushedPosition(where);
                   mappedFileLast.setWrotePosition(where);
                   mappedFileLast.setCommittedPosition(where);
                   break;
               } else {
                   iterator.remove();
               }
           }
           return true;
       }
   ```
   
   and I saws  deleteLastFile()  actually deleted the file.
   
   ```
       public void deleteLastMappedFile() {
           MappedFile lastMappedFile = getLastMappedFile();
           if (lastMappedFile != null) {
               lastMappedFile.destroy(1000);
               this.mappedFiles.remove(lastMappedFile);
               log.info("on recover, destroy a logic mapped file " + lastMappedFile.getFileName());
           }
       }
   ```
   
   So it appears to me that when we remove a mappedFile from `this.mappedFiles`，we actually want to delete the underlying file as well. I thought if we use `iterator.remove()` without deleting the file, we lose the reference to that file as well. (because this.mappedFiles are supposed to hold the )
   
   Otherwise it might be that we don't need to call `iterator.remove()`, because I saw Java 8 doc
   
   > (For CopyOnWriteArrayList iterators, in our case this.mappedFiles )Mutation operations on iterators (remove, set, and add) are not supported. These methods throw UnsupportedOperationException. (which is a runtime exception)
   
   Which seems weird to me. If that's true then the `iterator.remove()` is not working at all.
   
   
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org