[
https://issues.apache.org/jira/browse/ROCKETMQ-23?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15783206#comment-15783206
]
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_r94050231
--- 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 --
right, that's what I also thought and ask above about `RETRY_TIMES_OVER`.
Thank you for clarifying this.
How about this (flushing until data is available and `MAX_FLUSH_NUM` (which
can be quite large) is reached, and log warnings when cannot flush)?
```java
for (int i = 0; i < MAX_FLUSH_NUM && remainHowManyDataToFlush() > 0; i++) {
if (!CommitLog.this.mappedFileQueue.flush(0))
CommitLog.log.warn(this.getServiceName() + " service shutdown,
failed flush retried "
+ (i + 1) + " times");
}
CommitLog.log.info(this.getServiceName() + " service shutdown, all data is
flushed successfully");
```
> 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)
