Uploaded image for project: 'Accumulo'
  1. Accumulo
  2. ACCUMULO-4148

Native map should increment counter for every cell

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.5, 1.7.0
    • Fix Version/s: 1.6.6, 1.7.2, 1.8.0
    • Component/s: None
    • Labels:
      None

      Description

      Dan Blum uncovered a bug in native map outlined in a mailing list thread.

      If the link to the thread stops working, below is the header for the first message in the thread to use for searching.

      From 	"Dan Blum" <db...@bbn.com>
      Subject 	Bug in either InMemoryMap or NativeMap
      Date 	Fri, 19 Feb 2016 20:34:59 GMT
      

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user mjwall opened a pull request:

          https://github.com/apache/accumulo/pull/82

          4148 inmemorymap counter

          Tests and fixes for ACCUMULO-4148

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

          $ git pull https://github.com/mjwall/accumulo 4148-inmemorymap-counter

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

          https://github.com/apache/accumulo/pull/82.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 #82


          commit a7a738f3d92d071ac2507fbfce2e71695b82cc81
          Author: Michael Wall <mjwall@gmail.com>
          Date: 2016-02-26T02:20:15Z

          ACCUMULO-4148 Explaination of problem, with tests

          The problem here is that InMemoryMap$DefaultMap increments the mutationCount or
          kvCount for every key value pair in every Mutation that is passed in. The
          NativeMap, which is used by the InMemoryMap$NativeMapWrapper does not. This
          causes 2 different issues in the NativeMap.

          1) When a single Mutation has duplicate key value pairs, only the last is
          recorded, because they all have the same mutationCount and the earlier ones are
          hidden.
          2 ) When multiple Mutations are passed in at the same time, the mutationCount
          or kvCount starts over for each Mutation. This can also lead to hiding of key
          value pairs.

          The tests added here expose both the issues as well as do some asserts on
          simple Mutations. A few tweaks were made to expose information to these tests.

          1) Made MemKey public, made it's kvCount private and exposed that via a getter so
          we can inspect directly instead of parsing the toString. Required changing
          some calls to kvCount to use the getter.

          2) Added a final String to the InMemoryMap which is set during construction. This
          allows you to see what kind of SimpleMap was setup in the InMemoryMap.

          Typically, mulitple asserts in one test are not the best design. But in this case
          so much setup was required and I wanted to compare how different InMemoryMaps
          treated the same collection of mutations.

          Here is the output from running these tests

          Failed tests:
          InMemoryMapIT.testMultipleMutationsMultipleKeysSomeSame:192->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper
          a 8cf:8cq [] 0 false mc=2
          a 8cf:8cq [] 0 false mc=1
          a 8cf1:8cq1 [] 0 false mc=2
          a 8cf1:8cq1 [] 0 false mc=1
          a 8cf2:8cq2 [] 0 false mc=2
          a 8cf2:8cq2 [] 0 false mc=1
          a 8cf3:8cq3 [] 0 false mc=2
          b 8cf1:8cq1 [] 0 false mc=3
          b 8cf2:8cq2 [] 0 false mc=3
          expected:<11> but was:<9>
          InMemoryMapIT.testMultipleMutationsMultipleSameKeys:165->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper
          a 7cf:7cq [] 0 false mc=2
          a 7cf:7cq [] 0 false mc=1
          expected:<5> but was:<2>
          InMemoryMapIT.testOneMutationManyKeys:106->assertEquivalentMutate:196->assertEquivalentMutate:221->assertMutatesEquivalent:250 InMemoryMap did not have distinct kvCounts InMemoryMap type NativeMapWrapper
          a 2cf1:2cq1 [] 0 false mc=1
          a 2cf2:2cq2 [] 0 false mc=1
          a 2cf3:2cq3 [] 0 false mc=1
          a 2cf4:2cq4 [] 0 false mc=1
          a 2cf5:2cq5 [] 0 false mc=1
          expected:<5> but was:<1>
          InMemoryMapIT.testOneMutationManySameKeys:117->assertEquivalentMutate:196->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper
          a 3cf:3cq [] 0 false mc=1
          expected:<5> but was:<1>
          InMemoryMapIT.testMutlipleMutationsMultipleKeys:151->assertEquivalentMutate:221->assertMutatesEquivalent:250 InMemoryMap did not have distinct kvCounts InMemoryMap type NativeMapWrapper
          a 6cf1:6cq1 [] 0 false mc=1
          a 6cf2:6cq2 [] 0 false mc=1
          a 6cf3:6cq3 [] 0 false mc=1
          a 6cf4:6cq4 [] 0 false mc=1
          a 6cf5:6cq5 [] 0 false mc=1
          b 6cf1:6cq1 [] 0 false mc=2
          b 6cf2:6cq2 [] 0 false mc=2
          expected:<7> but was:<2>

          Tests run: 8, Failures: 5, Errors: 0, Skipped: 0

          commit 3da5412de0cc08133dec7215ce8fe861f73219e4
          Author: Michael Wall <mjwall@gmail.com>
          Date: 2016-03-20T15:04:06Z

          ACCUMULO-4148 Fix one Mutation with duplicate keys

          Make NativeMap increment the kvCount each time a key pair is mutated. This is
          what happens in the mutate method of InMemoryMap$DefaultMap, the kvCount++.

          Still have failures though when multiple mutations are passed at the same time. Here is the output:

          Failed tests:
          InMemoryMapIT.testMultipleMutationsMultipleKeysSomeSame:192->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper
          a 8cf:8cq [] 0 false mc=3
          a 8cf:8cq [] 0 false mc=2
          a 8cf:8cq [] 0 false mc=1
          a 8cf1:8cq1 [] 0 false mc=4
          a 8cf1:8cq1 [] 0 false mc=2
          a 8cf2:8cq2 [] 0 false mc=5
          a 8cf2:8cq2 [] 0 false mc=3
          a 8cf3:8cq3 [] 0 false mc=6
          b 8cf1:8cq1 [] 0 false mc=3
          b 8cf2:8cq2 [] 0 false mc=4
          expected:<11> but was:<10>
          InMemoryMapIT.testMultipleMutationsMultipleSameKeys:165->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper
          a 7cf:7cq [] 0 false mc=4
          a 7cf:7cq [] 0 false mc=3
          a 7cf:7cq [] 0 false mc=2
          a 7cf:7cq [] 0 false mc=1
          expected:<5> but was:<4>
          InMemoryMapIT.testMutlipleMutationsMultipleKeys:151->assertEquivalentMutate:221->assertMutatesEquivalent:250 InMemoryMap did not have distinct kvCounts InMemoryMap type NativeMapWrapper
          a 6cf1:6cq1 [] 0 false mc=1
          a 6cf2:6cq2 [] 0 false mc=2
          a 6cf3:6cq3 [] 0 false mc=3
          a 6cf4:6cq4 [] 0 false mc=4
          a 6cf5:6cq5 [] 0 false mc=5
          b 6cf1:6cq1 [] 0 false mc=2
          b 6cf2:6cq2 [] 0 false mc=3
          expected:<7> but was:<5>

          commit 80206d5e4c7669321b2b9d7971dd45f60d4c45ae
          Author: Michael Wall <mjwall@gmail.com>
          Date: 2016-03-20T15:14:12Z

          ACCUMULO-4148 Fix for multiple Mutation objects

          This is the fix Keith original recommended in the email thread. Make
          NativeMap._mutate return the current kvCount so it can be used for every
          Mutation that is passed in.

          Also, there were 2 different code paths for NativeMap methods

          public void mutate(Mutation mutation, int mutationCount)

          and

          public void mutate(List<Mutation> mutations, int mutationCount)

          so let's make the first call the second.

          All the InMemoryMapIT tests are passing.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user mjwall opened a pull request: https://github.com/apache/accumulo/pull/82 4148 inmemorymap counter Tests and fixes for ACCUMULO-4148 You can merge this pull request into a Git repository by running: $ git pull https://github.com/mjwall/accumulo 4148-inmemorymap-counter Alternatively you can review and apply these changes as the patch at: https://github.com/apache/accumulo/pull/82.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 #82 commit a7a738f3d92d071ac2507fbfce2e71695b82cc81 Author: Michael Wall <mjwall@gmail.com> Date: 2016-02-26T02:20:15Z ACCUMULO-4148 Explaination of problem, with tests The problem here is that InMemoryMap$DefaultMap increments the mutationCount or kvCount for every key value pair in every Mutation that is passed in. The NativeMap, which is used by the InMemoryMap$NativeMapWrapper does not. This causes 2 different issues in the NativeMap. 1) When a single Mutation has duplicate key value pairs, only the last is recorded, because they all have the same mutationCount and the earlier ones are hidden. 2 ) When multiple Mutations are passed in at the same time, the mutationCount or kvCount starts over for each Mutation. This can also lead to hiding of key value pairs. The tests added here expose both the issues as well as do some asserts on simple Mutations. A few tweaks were made to expose information to these tests. 1) Made MemKey public, made it's kvCount private and exposed that via a getter so we can inspect directly instead of parsing the toString. Required changing some calls to kvCount to use the getter. 2) Added a final String to the InMemoryMap which is set during construction. This allows you to see what kind of SimpleMap was setup in the InMemoryMap. Typically, mulitple asserts in one test are not the best design. But in this case so much setup was required and I wanted to compare how different InMemoryMaps treated the same collection of mutations. Here is the output from running these tests Failed tests: InMemoryMapIT.testMultipleMutationsMultipleKeysSomeSame:192->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper a 8cf:8cq [] 0 false mc=2 a 8cf:8cq [] 0 false mc=1 a 8cf1:8cq1 [] 0 false mc=2 a 8cf1:8cq1 [] 0 false mc=1 a 8cf2:8cq2 [] 0 false mc=2 a 8cf2:8cq2 [] 0 false mc=1 a 8cf3:8cq3 [] 0 false mc=2 b 8cf1:8cq1 [] 0 false mc=3 b 8cf2:8cq2 [] 0 false mc=3 expected:<11> but was:<9> InMemoryMapIT.testMultipleMutationsMultipleSameKeys:165->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper a 7cf:7cq [] 0 false mc=2 a 7cf:7cq [] 0 false mc=1 expected:<5> but was:<2> InMemoryMapIT.testOneMutationManyKeys:106->assertEquivalentMutate:196->assertEquivalentMutate:221->assertMutatesEquivalent:250 InMemoryMap did not have distinct kvCounts InMemoryMap type NativeMapWrapper a 2cf1:2cq1 [] 0 false mc=1 a 2cf2:2cq2 [] 0 false mc=1 a 2cf3:2cq3 [] 0 false mc=1 a 2cf4:2cq4 [] 0 false mc=1 a 2cf5:2cq5 [] 0 false mc=1 expected:<5> but was:<1> InMemoryMapIT.testOneMutationManySameKeys:117->assertEquivalentMutate:196->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper a 3cf:3cq [] 0 false mc=1 expected:<5> but was:<1> InMemoryMapIT.testMutlipleMutationsMultipleKeys:151->assertEquivalentMutate:221->assertMutatesEquivalent:250 InMemoryMap did not have distinct kvCounts InMemoryMap type NativeMapWrapper a 6cf1:6cq1 [] 0 false mc=1 a 6cf2:6cq2 [] 0 false mc=1 a 6cf3:6cq3 [] 0 false mc=1 a 6cf4:6cq4 [] 0 false mc=1 a 6cf5:6cq5 [] 0 false mc=1 b 6cf1:6cq1 [] 0 false mc=2 b 6cf2:6cq2 [] 0 false mc=2 expected:<7> but was:<2> Tests run: 8, Failures: 5, Errors: 0, Skipped: 0 commit 3da5412de0cc08133dec7215ce8fe861f73219e4 Author: Michael Wall <mjwall@gmail.com> Date: 2016-03-20T15:04:06Z ACCUMULO-4148 Fix one Mutation with duplicate keys Make NativeMap increment the kvCount each time a key pair is mutated. This is what happens in the mutate method of InMemoryMap$DefaultMap, the kvCount++. Still have failures though when multiple mutations are passed at the same time. Here is the output: Failed tests: InMemoryMapIT.testMultipleMutationsMultipleKeysSomeSame:192->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper a 8cf:8cq [] 0 false mc=3 a 8cf:8cq [] 0 false mc=2 a 8cf:8cq [] 0 false mc=1 a 8cf1:8cq1 [] 0 false mc=4 a 8cf1:8cq1 [] 0 false mc=2 a 8cf2:8cq2 [] 0 false mc=5 a 8cf2:8cq2 [] 0 false mc=3 a 8cf3:8cq3 [] 0 false mc=6 b 8cf1:8cq1 [] 0 false mc=3 b 8cf2:8cq2 [] 0 false mc=4 expected:<11> but was:<10> InMemoryMapIT.testMultipleMutationsMultipleSameKeys:165->assertEquivalentMutate:221->assertMutatesEquivalent:247 Not all key value pairs included: InMemoryMap type NativeMapWrapper a 7cf:7cq [] 0 false mc=4 a 7cf:7cq [] 0 false mc=3 a 7cf:7cq [] 0 false mc=2 a 7cf:7cq [] 0 false mc=1 expected:<5> but was:<4> InMemoryMapIT.testMutlipleMutationsMultipleKeys:151->assertEquivalentMutate:221->assertMutatesEquivalent:250 InMemoryMap did not have distinct kvCounts InMemoryMap type NativeMapWrapper a 6cf1:6cq1 [] 0 false mc=1 a 6cf2:6cq2 [] 0 false mc=2 a 6cf3:6cq3 [] 0 false mc=3 a 6cf4:6cq4 [] 0 false mc=4 a 6cf5:6cq5 [] 0 false mc=5 b 6cf1:6cq1 [] 0 false mc=2 b 6cf2:6cq2 [] 0 false mc=3 expected:<7> but was:<5> commit 80206d5e4c7669321b2b9d7971dd45f60d4c45ae Author: Michael Wall <mjwall@gmail.com> Date: 2016-03-20T15:14:12Z ACCUMULO-4148 Fix for multiple Mutation objects This is the fix Keith original recommended in the email thread. Make NativeMap._mutate return the current kvCount so it can be used for every Mutation that is passed in. Also, there were 2 different code paths for NativeMap methods public void mutate(Mutation mutation, int mutationCount) and public void mutate(List<Mutation> mutations, int mutationCount) so let's make the first call the second. All the InMemoryMapIT tests are passing.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/accumulo/pull/82

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/accumulo/pull/82
          Hide
          elserj Josh Elser added a comment -

          Thanks for the great work here, Michael Wall!

          I was working through some merge issues with Mike in IRC this morning. I ended up finding two test issues, and suggested that he open a follow-on issue if he'd like to address them (not problems, just tests that weren't exactly what he was intending).

          Show
          elserj Josh Elser added a comment - Thanks for the great work here, Michael Wall ! I was working through some merge issues with Mike in IRC this morning. I ended up finding two test issues, and suggested that he open a follow-on issue if he'd like to address them (not problems, just tests that weren't exactly what he was intending).

            People

            • Assignee:
              mjwall Michael Wall
              Reporter:
              kturner Keith Turner
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

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

                  Development