You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by fuyou001 <gi...@git.apache.org> on 2017/08/28 03:15:07 UTC

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

GitHub user fuyou001 opened a pull request:

    https://github.com/apache/incubator-rocketmq/pull/159

    [ROCKETMQ-279]  commit log data and consume queue data logic consistent

    To avoid commit log  and consume queue logic inconsistent .
    Motivation:
     add a check when mq start,if commit log  and consume queue logic inconsistent ,start failed.
    
    fast failed if check failed.
    
    (cherry picked from commit 6e77be0)

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/fuyou001/incubator-rocketmq ROCKETMQ-2792

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-rocketmq/pull/159.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #159
    
----
commit 0f0dfd35367402aea5ca3f40199387dd48cb1e7e
Author: 傅冲 <yu...@alibaba-inc.com>
Date:   2017-08-28T02:21:49Z

    [ROCKETMQ-279]  commit log data and consume queue data logic consistent
    
    (cherry picked from commit 6e77be0)

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #159: [ROCKETMQ-279] commit log data and consume qu...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/159
  
    
    [![Coverage Status](https://coveralls.io/builds/13028464/badge)](https://coveralls.io/builds/13028464)
    
    Coverage decreased (-0.4%) to 38.427% when pulling **1ddd284118133cbb21be3076b73153131d9c7167 on fuyou001:ROCKETMQ-2792** into **629c3e9ea084d0a2947b2ab6f1cc8b0c6ed98502 on apache:develop**.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/159#discussion_r136490566
  
    --- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java ---
    @@ -84,6 +84,58 @@ public boolean load() {
             return result;
         }
     
    +    public void checkCommitLogAndConsumeQueueConsistent() {
    +
    +        String queueDir = this.storePath
    --- End diff --
    
    There is a helper to get queue dir: `StorePathConfigHelper#getStorePathConsumeQueue`


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

Posted by fuyou001 <gi...@git.apache.org>.
Github user fuyou001 closed the pull request at:

    https://github.com/apache/incubator-rocketmq/pull/159


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #159: [ROCKETMQ-279] commit log data and consume qu...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/159
  
    
    [![Coverage Status](https://coveralls.io/builds/13011345/badge)](https://coveralls.io/builds/13011345)
    
    Coverage decreased (-0.09%) to 38.731% when pulling **bff62f4fd84b0084c655b05698c484381ee4e77f on fuyou001:ROCKETMQ-2792** into **629c3e9ea084d0a2947b2ab6f1cc8b0c6ed98502 on apache:develop**.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #159: [ROCKETMQ-279] commit log data and consume qu...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/159
  
    
    [![Coverage Status](https://coveralls.io/builds/13028470/badge)](https://coveralls.io/builds/13028470)
    
    Coverage decreased (-0.4%) to 38.468% when pulling **1ddd284118133cbb21be3076b73153131d9c7167 on fuyou001:ROCKETMQ-2792** into **629c3e9ea084d0a2947b2ab6f1cc8b0c6ed98502 on apache:develop**.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #159: [ROCKETMQ-279] commit log data and consume qu...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/159
  
    
    [![Coverage Status](https://coveralls.io/builds/13082255/badge)](https://coveralls.io/builds/13082255)
    
    Coverage decreased (-0.2%) to 38.511% when pulling **4b98ecac5ae00933cab648dc7b3446ea3c88e1bd on fuyou001:ROCKETMQ-2792** into **845830865fc37d0364a19cbd89ceaf8a30b37e1c on apache:develop**.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/159#discussion_r136491053
  
    --- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java ---
    @@ -84,6 +84,58 @@ public boolean load() {
             return result;
         }
     
    +    public void checkCommitLogAndConsumeQueueConsistent() {
    +
    +        String queueDir = this.storePath
    +            + File.separator + topic
    +            + File.separator + queueId + File.separator;
    +
    +        long lastRecordOffset = Math.max(getMaxOffsetInQueue() - 1, 0);
    +
    +        if (lastRecordOffset == 0) {
    +            return;
    +        }
    +
    +        SelectMappedBufferResult consumeQueueBuffer = getIndexBuffer(lastRecordOffset);
    +
    +        if (consumeQueueBuffer == null) {
    +            String errorMsg = String.format("Can't find consume queue mappedBuffer by offset %s,queueDir %s", lastRecordOffset, queueDir);
    +            log.error(errorMsg);
    +            throw new IllegalStateException(errorMsg);
    +        }
    +
    +        //position latest consume queue offset,
    +        long offsetPy = consumeQueueBuffer.getByteBuffer().getLong();
    +        int sizePy = consumeQueueBuffer.getByteBuffer().getInt();
    +        consumeQueueBuffer.getByteBuffer().getLong();
    --- End diff --
    
    Do we need the side effect of `getLong`?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/159#discussion_r135688453
  
    --- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java ---
    @@ -84,6 +84,59 @@ public boolean load() {
             return result;
         }
     
    +    public void checkCommitLogAndConsumeQueueConsistent() {
    +
    +        String queueDir = this.storePath
    +            + File.separator + topic
    +            + File.separator + queueId + File.separator;
    +
    +        long maxOffsetInQueue = getMaxOffsetInQueue();
    --- End diff --
    
    I would like to use `long lastRecordOffset = getMaxOffsetInQueue();` instead, to avoid using too many `+1 or -1` in below code block. Actually, `getMaxOffsetInQueue()` returns the next write position, not the position of last consume queue record.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

Posted by fuyou001 <gi...@git.apache.org>.
Github user fuyou001 commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/159#discussion_r135692614
  
    --- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java ---
    @@ -84,6 +84,59 @@ public boolean load() {
             return result;
         }
     
    +    public void checkCommitLogAndConsumeQueueConsistent() {
    +
    +        String queueDir = this.storePath
    +            + File.separator + topic
    +            + File.separator + queueId + File.separator;
    +
    +        long maxOffsetInQueue = getMaxOffsetInQueue();
    --- End diff --
    
    refactor


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/159#discussion_r135689182
  
    --- Diff: store/src/test/java/org/apache/rocketmq/store/ConsumeQueueTest.java ---
    @@ -207,4 +226,34 @@ public void dispatch(DispatchRequest request) {
                 UtilAll.deleteFile(new File(storePath));
             }
         }
    +
    +    @Test
    +    public void test_checkCommitLogAndConsumeQueueConsistent() throws Exception {
    +        DefaultMessageStore master = gen();
    +
    +        try {
    +            master.getDispatcherList().addFirst(new CommitLogDispatcher() {
    +
    +                @Override
    +                public void dispatch(DispatchRequest request) {
    +                    runCount++;
    +                }
    +
    +                private int runCount = 0;
    +            });
    +
    +            putMsg(master);
    +            // wait build consume queue
    +            Thread.sleep(2000);
    +
    +            ConsumeQueue cq = master.getConsumeQueueTable().get(topic).get(queueId);
    +            cq.checkCommitLogAndConsumeQueueConsistent();
    --- End diff --
    
    It seems that this test only covers the consistent condition? 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...

Posted by fuyou001 <gi...@git.apache.org>.
Github user fuyou001 commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/159#discussion_r136494489
  
    --- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java ---
    @@ -84,6 +84,58 @@ public boolean load() {
             return result;
         }
     
    +    public void checkCommitLogAndConsumeQueueConsistent() {
    +
    +        String queueDir = this.storePath
    +            + File.separator + topic
    +            + File.separator + queueId + File.separator;
    +
    +        long lastRecordOffset = Math.max(getMaxOffsetInQueue() - 1, 0);
    +
    +        if (lastRecordOffset == 0) {
    +            return;
    +        }
    +
    +        SelectMappedBufferResult consumeQueueBuffer = getIndexBuffer(lastRecordOffset);
    +
    +        if (consumeQueueBuffer == null) {
    +            String errorMsg = String.format("Can't find consume queue mappedBuffer by offset %s,queueDir %s", lastRecordOffset, queueDir);
    +            log.error(errorMsg);
    +            throw new IllegalStateException(errorMsg);
    +        }
    +
    +        //position latest consume queue offset,
    +        long offsetPy = consumeQueueBuffer.getByteBuffer().getLong();
    +        int sizePy = consumeQueueBuffer.getByteBuffer().getInt();
    +        consumeQueueBuffer.getByteBuffer().getLong();
    --- End diff --
    
    fixed


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---