[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 @shroman It seems that last cleaning, for too long-time no-resolved prs, included this one. If you feel it is necessary to reopen this PR, please feel free to do it. you may also could create a new PR or launch new discussion in the mailing list. ---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 And why is it closed? ---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 @shroman could you please resolve the conflicts? IMO, the return value of flush() and commit(), means whether there is data left to be flushed or committed, that is OK. if you'd like to polish the semantic of the returned value, may you do the same polishing for commit() too, just to keep the same semantic for similar methods. ---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 Ok, guys, I set the fix version of the JIRA issue to 4.1.0 then ð @vongosling what are your recommendations on how to treat the storage module? Should I create a JIRA issue if I find a problem, but not work on it? Or should I create a PR, but it would be merged only in 4.1.0? Just curious, why merging the fixes to it is better for later versions? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 We must keep cautious about storage polish, although some minor rename. IMO, Could we optimize here in the 4.1.0 or more later version :-) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/20
@zhouxinyu I made changes according to your comments, and added more tests.
In {{CommitLog.FlushRealTimeService#run()}}, we will try to flush as long
as the data is available or {{MAX_FLUSH_COMMIT_NUM}} is not reached, logging
when flushing fails. It is almost same as before (checking the return value of
{{flush()}}) but checking if looks neat to me. What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 Hi, the github says "This branch has no conflicts with the base branch" I think conflicts are resolved. Reviewing it by several committers is already being cautious, isn't it? ;) This issue looks much like a pretty nasty bug. Anyway, looking forward to your reviews ;) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 Hi, shroman Could you please resolve conflicts in this pr? And we will remain cautious about the changes of rocketmq-store module, so this pr may be won't merge quickly. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...
Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 @vongosling Does `RETRY_TIMES_OVER` here means retries on failures, or the number of times to flush on success? I think the former, but need to be sure I understand the intentions of the person who wrote the code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
