[
https://issues.apache.org/jira/browse/ROCKETMQ-23?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15783125#comment-15783125
]
ASF GitHub Bot commented on ROCKETMQ-23:
----------------------------------------
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/20#discussion_r94045317
--- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java ---
@@ -964,9 +964,11 @@ public void run() {
boolean result = false;
for (int i = 0; i < RETRY_TIMES_OVER && !result; i++) {
--- End diff --
Consider `result = flush(0)`, the false result means there may be more data
need to flush, so retry. Otherwise, the true result means all data are flushed,
the broker can exit safely.
So if you change `result`, also please change this `for` loop.
> 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)
