[ 
https://issues.apache.org/jira/browse/ROCKETMQ-23?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15783134#comment-15783134
 ] 

ASF GitHub Bot commented on ROCKETMQ-23:
----------------------------------------

Github user shroman commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/20#discussion_r94045874
  
    --- Diff: 
store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java ---
    @@ -39,11 +39,15 @@
     
         private final int mappedFileSize;
     
    -    private final CopyOnWriteArrayList<MappedFile> mappedFiles = new 
CopyOnWriteArrayList<MappedFile>();
    +    private final CopyOnWriteArrayList<MappedFile> mappedFiles = new 
CopyOnWriteArrayList<>();
     
         private final AllocateMappedFileService allocateMappedFileService;
     
    -    private long flushedWhere = 0;
    --- End diff --
    
    yes, I understand this, but `flushedWhere` doesn't sound good. File's 
offset is the file's offset, but this offset is the offset withing the queue of 
all files ;) Should we rename it to `flushedPosition`? @zhouxinyu What do you 
think?


> MappedFileQueue#flush should return true when flushing is successful
> --------------------------------------------------------------------
>
>                 Key: ROCKETMQ-23
>                 URL: https://issues.apache.org/jira/browse/ROCKETMQ-23
>             Project: Apache RocketMQ
>          Issue Type: Bug
>          Components: rocketmq-store
>    Affects Versions: 4.0.0-incubating
>            Reporter: Roman Shtykh
>            Assignee: Roman Shtykh
>
> In the current implementation, MappedFileQueue#flush returns {{false}} when 
> flushing is successful.
> This is not intuitive and error prone.
> For instance, in {{CommitLog#run line:915-918}}
> {code}
> for (int i = 0; i < RETRY_TIMES_OVER && !result; i++) {
>    result = CommitLog.this.mappedFileQueue.flush(0);
>    // ...
> }
> {code}
> I believe retries has to be done when flushing is not successful. But with 
> the code above, it can try to flush only once on CommitLog termination and 
> not continue retrying.
> The same is for {{DefaultMessageStore#doFlush line:1551}}
> Or is this not retry on failure, but the number of times flushing has to be 
> done? Then, {{RETRY_TIMES_OVER}} should be renamed to something like 
> {{FLUSH_NUM}}.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to