Uploaded image for project: 'Apache RocketMQ'
  1. Apache RocketMQ
  2. ROCKETMQ-74

DataVersion equals not working as expected.

    Details

      Description

      DataVersion equals method assumes that AtomicLong's equals method comparing by value. This assumption turns out invalid.

      This brings about a few issues, say, wipeWritePerm command would not be working as expected as perm will be overridden in 30s at most.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user lizhanhui opened a pull request:

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

          ROCKETMQ-74 Fix DataVersion equals defect

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

          $ git pull https://github.com/lizhanhui/incubator-rocketmq ROCKETMQ-74

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

          https://github.com/apache/incubator-rocketmq/pull/50.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 #50


          commit 0a7ce6069a5dff6b5858bfe13ba984655870c464
          Author: Zhanhui Li <lizhanhui@apache.org>
          Date: 2017-01-24T10:24:48Z

          Fix equals defect


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user lizhanhui opened a pull request: https://github.com/apache/incubator-rocketmq/pull/50 ROCKETMQ-74 Fix DataVersion equals defect You can merge this pull request into a Git repository by running: $ git pull https://github.com/lizhanhui/incubator-rocketmq ROCKETMQ-74 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/50.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 #50 commit 0a7ce6069a5dff6b5858bfe13ba984655870c464 Author: Zhanhui Li <lizhanhui@apache.org> Date: 2017-01-24T10:24:48Z Fix equals defect
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9814272/badge)](https://coveralls.io/builds/9814272)

          Coverage increased (+0.09%) to 30.163% when pulling *c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9814272/badge)](https://coveralls.io/builds/9814272 ) Coverage increased (+0.09%) to 30.163% when pulling * c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9814272/badge)](https://coveralls.io/builds/9814272)

          Coverage increased (+0.09%) to 30.163% when pulling *c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9814272/badge)](https://coveralls.io/builds/9814272 ) Coverage increased (+0.09%) to 30.163% when pulling * c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9814272/badge)](https://coveralls.io/builds/9814272)

          Coverage increased (+0.09%) to 30.163% when pulling *c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9814272/badge)](https://coveralls.io/builds/9814272 ) Coverage increased (+0.09%) to 30.163% when pulling * c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9814692/badge)](https://coveralls.io/builds/9814692)

          Coverage increased (+0.2%) to 30.285% when pulling *c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9814692/badge)](https://coveralls.io/builds/9814692 ) Coverage increased (+0.2%) to 30.285% when pulling * c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9814692/badge)](https://coveralls.io/builds/9814692)

          Coverage increased (+0.2%) to 30.285% when pulling *c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9814692/badge)](https://coveralls.io/builds/9814692 ) Coverage increased (+0.2%) to 30.285% when pulling * c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9814692/badge)](https://coveralls.io/builds/9814692)

          Coverage increased (+0.2%) to 30.285% when pulling *c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9814692/badge)](https://coveralls.io/builds/9814692 ) Coverage increased (+0.2%) to 30.285% when pulling * c6016ec3b081f35da1e9766244905a1791df09dd on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9818406/badge)](https://coveralls.io/builds/9818406)

          Coverage increased (+0.2%) to 30.25% when pulling *1a4399b1b2b26698bc051b3832776f76c2916ebf on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9818406/badge)](https://coveralls.io/builds/9818406 ) Coverage increased (+0.2%) to 30.25% when pulling * 1a4399b1b2b26698bc051b3832776f76c2916ebf on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9818406/badge)](https://coveralls.io/builds/9818406)

          Coverage increased (+0.2%) to 30.25% when pulling *1a4399b1b2b26698bc051b3832776f76c2916ebf on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9818406/badge)](https://coveralls.io/builds/9818406 ) Coverage increased (+0.2%) to 30.25% when pulling * 1a4399b1b2b26698bc051b3832776f76c2916ebf on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9818406/badge)](https://coveralls.io/builds/9818406)

          Coverage increased (+0.2%) to 30.25% when pulling *1a4399b1b2b26698bc051b3832776f76c2916ebf on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9818406/badge)](https://coveralls.io/builds/9818406 ) Coverage increased (+0.2%) to 30.25% when pulling * 1a4399b1b2b26698bc051b3832776f76c2916ebf on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/incubator-rocketmq/pull/50#discussion_r97704748

          — Diff: common/src/main/java/org/apache/rocketmq/common/DataVersion.java —
          @@ -58,16 +58,28 @@ public boolean equals(final Object o) {

          final DataVersion that = (DataVersion) o;

          • if (timestatmp != that.timestatmp)
            + if (timestamp != that.timestamp)
            return false;
          • return counter != null ? counter.equals(that.counter) : that.counter == null;
            +
            + if (null == counter && null == that.counter) { + return true; + }

            +
            + if (counter != null && that.counter == null || counter == null)

            { + return false; + }

            +

              • End diff –

          I have a advise, when compare the `counter`, below code is more readable:

          ```java
          if (counter != null && that.counter != null)

          { return counter.longValue() == that.counter.longValue(); }

          return counter == that.counter;
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/50#discussion_r97704748 — Diff: common/src/main/java/org/apache/rocketmq/common/DataVersion.java — @@ -58,16 +58,28 @@ public boolean equals(final Object o) { final DataVersion that = (DataVersion) o; if (timestatmp != that.timestatmp) + if (timestamp != that.timestamp) return false; return counter != null ? counter.equals(that.counter) : that.counter == null; + + if (null == counter && null == that.counter) { + return true; + } + + if (counter != null && that.counter == null || counter == null) { + return false; + } + End diff – I have a advise, when compare the `counter`, below code is more readable: ```java if (counter != null && that.counter != null) { return counter.longValue() == that.counter.longValue(); } return counter == that.counter; ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhouxinyu commented on the issue:

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

          Please @vongosling @lollipopjin help review.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 Please @vongosling @lollipopjin help review.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lollipopjin commented on the issue:

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

          Seems ok.

          Show
          githubbot ASF GitHub Bot added a comment - Github user lollipopjin commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 Seems ok.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9885555/badge)](https://coveralls.io/builds/9885555)

          Coverage increased (+0.1%) to 30.181% when pulling *e01c72978b9bb5812f150595cbd0ba5ca6a6ad4e on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9885555/badge)](https://coveralls.io/builds/9885555 ) Coverage increased (+0.1%) to 30.181% when pulling * e01c72978b9bb5812f150595cbd0ba5ca6a6ad4e on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9885555/badge)](https://coveralls.io/builds/9885555)

          Coverage increased (+0.1%) to 30.181% when pulling *e01c72978b9bb5812f150595cbd0ba5ca6a6ad4e on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9885555/badge)](https://coveralls.io/builds/9885555 ) Coverage increased (+0.1%) to 30.181% when pulling * e01c72978b9bb5812f150595cbd0ba5ca6a6ad4e on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

          [![Coverage Status](https://coveralls.io/builds/9885555/badge)](https://coveralls.io/builds/9885555)

          Coverage increased (+0.1%) to 30.181% when pulling *e01c72978b9bb5812f150595cbd0ba5ca6a6ad4e on lizhanhui:ROCKETMQ-74* into *30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 [! [Coverage Status] ( https://coveralls.io/builds/9885555/badge)](https://coveralls.io/builds/9885555 ) Coverage increased (+0.1%) to 30.181% when pulling * e01c72978b9bb5812f150595cbd0ba5ca6a6ad4e on lizhanhui: ROCKETMQ-74 * into * 30cabc77bd99abcf934d822d69030ddcfe38f94a on apache:master *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lizhanhui commented on the issue:

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

          @vongosling Any opinion on this PR? If not, I would merge it to master branch.

          Show
          githubbot ASF GitHub Bot added a comment - Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 @vongosling Any opinion on this PR? If not, I would merge it to master branch.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vongosling commented on the issue:

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

          @lizhanhui since 4.1.0, we will start a new branching model, more formalized, more flexible. All PRs except those bugfix, we recommended to merge back to develop branch. This guide will publish in website sooner

          Show
          githubbot ASF GitHub Bot added a comment - Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 @lizhanhui since 4.1.0, we will start a new branching model, more formalized, more flexible. All PRs except those bugfix, we recommended to merge back to develop branch. This guide will publish in website sooner
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user djKooks commented on the issue:

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

          LGTM!

          Show
          githubbot ASF GitHub Bot added a comment - Github user djKooks commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 LGTM!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vongosling commented on the issue:

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

          @djKooks You're amazing~ http://rocketmq.incubator.apache.org/docs/branching-model

          Show
          githubbot ASF GitHub Bot added a comment - Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 @djKooks You're amazing~ http://rocketmq.incubator.apache.org/docs/branching-model
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user djKooks commented on the issue:

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

          @vongosling Thanks for guide! I'll refer to it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user djKooks commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 @vongosling Thanks for guide! I'll refer to it.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 767775838fbbf556e6ebc899f4ccab7f148d7aae in incubator-rocketmq's branch refs/heads/master from Zhanhui Li
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=7677758 ]

          ROCKETMQ-74 Fix DataVersion equals defect, closes apache/incubator-rocketmq#50

          Show
          jira-bot ASF subversion and git services added a comment - Commit 767775838fbbf556e6ebc899f4ccab7f148d7aae in incubator-rocketmq's branch refs/heads/master from Zhanhui Li [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=7677758 ] ROCKETMQ-74 Fix DataVersion equals defect, closes apache/incubator-rocketmq#50
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/50
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 767775838fbbf556e6ebc899f4ccab7f148d7aae in incubator-rocketmq's branch refs/heads/release-4.0.0-incubating from Zhanhui Li
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=7677758 ]

          ROCKETMQ-74 Fix DataVersion equals defect, closes apache/incubator-rocketmq#50

          Show
          jira-bot ASF subversion and git services added a comment - Commit 767775838fbbf556e6ebc899f4ccab7f148d7aae in incubator-rocketmq's branch refs/heads/release-4.0.0-incubating from Zhanhui Li [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=7677758 ] ROCKETMQ-74 Fix DataVersion equals defect, closes apache/incubator-rocketmq#50

            People

            • Assignee:
              Yukon yukon
              Reporter:
              lizhanhui Zhanhui Li
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development