You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by mjwall <gi...@git.apache.org> on 2016/03/20 19:05:57 UTC

[GitHub] accumulo pull request: 4148 inmemorymap counter

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 <mj...@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 <mj...@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 <mj...@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.

----


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57589046
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,319 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    --- End diff --
    
    The -pl :accumulo-test means maven will only run the accumulo test module right?  If integration-test had not been run on the native submodule, the libaccumulo file would be missing.  Changing to the test directory was the other case.  Is there something I need to change here?


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56998269
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    --- End diff --
    
    nice background


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57574776
  
    --- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java ---
    @@ -202,10 +203,26 @@ public InMemoryMap(Map<String,Set<ByteSequence>> lggroups, boolean useNativeMap,
         this.memDumpDir = memDumpDir;
         this.lggroups = lggroups;
     
    -    if (lggroups.size() == 0)
    +    if (lggroups.size() == 0) {
           map = newMap(useNativeMap);
    -    else
    +      mapType = useNativeMap ? "NativeMapWrapper" : "DefaultMap";
    --- End diff --
    
    Using constants here would be nice (and could be re-used in your test)


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56997899
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, getUniqKVCount(memKeys1).size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, getUniqKVCount(memKeys2).size());
    +
    +  }
    +
    +  private int countKVPairs(List<Mutation> mutations) {
    +    int count = 0;
    +    for (Mutation m : mutations) {
    +      count += m.size();
    +    }
    +    return count;
    +  }
    +
    +  private List<MemKey> getArrayOfMemKeys(InMemoryMap imm) {
    +    SortedKeyValueIterator<Key,Value> skvi = imm.compactionIterator();
    +
    +    List<MemKey> memKeys = new ArrayList<MemKey>();
    +    try {
    +      skvi.seek(new Range(), new ArrayList<ByteSequence>(), false); // everything
    +      while (skvi.hasTop()) {
    +        memKeys.add((MemKey) skvi.getTopKey());
    +        skvi.next();
    +      }
    +    } catch (IOException ex) {
    +      java.util.logging.Logger.getLogger(InMemoryMapIT.class.getName()).log(Level.SEVERE, null, ex);
    --- End diff --
    
    Yep, good call.  That is an autocomplete I should have updated.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57590769
  
    --- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java ---
    @@ -202,10 +203,26 @@ public InMemoryMap(Map<String,Set<ByteSequence>> lggroups, boolean useNativeMap,
         this.memDumpDir = memDumpDir;
         this.lggroups = lggroups;
     
    -    if (lggroups.size() == 0)
    +    if (lggroups.size() == 0) {
           map = newMap(useNativeMap);
    -    else
    +      mapType = useNativeMap ? "NativeMapWrapper" : "DefaultMap";
    --- End diff --
    
    I just saw that `mapType()` was invoked in the test and assumed that there was a corresponding assertion on the value at some point. Thanks for making these constants.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57574698
  
    --- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java ---
    @@ -519,36 +520,25 @@ protected void finalize() throws Throwable {
         }
       }
     
    -  private void _mutate(Mutation mutation, int mutationCount) {
    +  private int _mutate(Mutation mutation, int mutationCount) {
     
         List<ColumnUpdate> updates = mutation.getUpdates();
         if (updates.size() == 1) {
           ColumnUpdate update = updates.get(0);
           singleUpdate(nmPointer, mutation.getRow(), update.getColumnFamily(), update.getColumnQualifier(), update.getColumnVisibility(), update.getTimestamp(),
    -          update.isDeleted(), update.getValue(), mutationCount);
    +          update.isDeleted(), update.getValue(), mutationCount++);
         } else if (updates.size() > 1) {
           long uid = startUpdate(nmPointer, mutation.getRow());
           for (ColumnUpdate update : updates) {
             update(nmPointer, uid, update.getColumnFamily(), update.getColumnQualifier(), update.getColumnVisibility(), update.getTimestamp(), update.isDeleted(),
    -            update.getValue(), mutationCount);
    +            update.getValue(), mutationCount++);
           }
    -
         }
    +    return mutationCount;
       }
     
       public void mutate(Mutation mutation, int mutationCount) {
    -    wlock.lock();
    --- End diff --
    
    Is this write lock removal because it was unnecessary? (`mutate(List, int)` was re-grabbing the same lock)


---
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] accumulo pull request: 4148 inmemorymap counter

Posted by mjwall <gi...@git.apache.org>.
Github user mjwall commented on the pull request:

    https://github.com/apache/accumulo/pull/82#issuecomment-203904254
  
    I'll make another branch where I change those constants final and squash the commits to one.  Then I'll try to merge to 1.6 locally.  If that works, I'll close this PR and make new one for 1.6 and do separate PRs for 1.7 and master to make this easier.  Thanks for looking at it @joshelser 


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56997973
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    --- End diff --
    
    Will this check always be true if the two before were true?


---
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] accumulo pull request: 4148 inmemorymap counter

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the pull request:

    https://github.com/apache/accumulo/pull/82#issuecomment-203248708
  
    I started merging this tonight, but am going to have to pick it up again later. The changes in 17b4b3c were bad because they weren't `final`. I've fixed those locally when squash-merge'ing. Some other easily fixed conflicts from 1.6->1.7. 1.7->master has some issues with the Sampler stuff @keith-turner has already added. I'll have to look into this one further to resolve.


---
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] accumulo pull request: 4148 inmemorymap counter

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the pull request:

    https://github.com/apache/accumulo/pull/82#issuecomment-203961544
  
    Don't worry about the squash and final changes, @mjwall, I already have them fixed locally (but thanks for offering). I already have 1.6 and 1.7 good. If you want to look at master and get that in a good place, that would be helpful (make sure I resolve the conflicts correctly).


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56999430
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, getUniqKVCount(memKeys1).size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, getUniqKVCount(memKeys2).size());
    +
    +  }
    +
    +  private int countKVPairs(List<Mutation> mutations) {
    +    int count = 0;
    +    for (Mutation m : mutations) {
    +      count += m.size();
    +    }
    +    return count;
    +  }
    +
    +  private List<MemKey> getArrayOfMemKeys(InMemoryMap imm) {
    +    SortedKeyValueIterator<Key,Value> skvi = imm.compactionIterator();
    +
    +    List<MemKey> memKeys = new ArrayList<MemKey>();
    +    try {
    +      skvi.seek(new Range(), new ArrayList<ByteSequence>(), false); // everything
    +      while (skvi.hasTop()) {
    +        memKeys.add((MemKey) skvi.getTopKey());
    +        skvi.next();
    +      }
    +    } catch (IOException ex) {
    +      java.util.logging.Logger.getLogger(InMemoryMapIT.class.getName()).log(Level.SEVERE, null, ex);
    +    }
    +
    +    return memKeys;
    +  }
    +
    +  private String dumpInMemoryMap(InMemoryMap map, List<MemKey> memkeys) {
    +    StringBuilder sb = new StringBuilder();
    +    sb.append("InMemoryMap type ");
    +    sb.append(map.getMapType());
    +    sb.append("\n");
    +
    +    for (MemKey mk : memkeys) {
    +      sb.append("  ");
    +      sb.append(mk.toString());
    +      sb.append("\n");
    +    }
    +
    +    return sb.toString();
    +  }
    +
    +  private List getUniqKVCount(List<MemKey> memKeys) {
    +    List<Integer> kvCounts = new ArrayList<Integer>();
    --- End diff --
    
    I should probably change the return type to Set<Integer>


---
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] accumulo pull request: 4148 inmemorymap counter

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the pull request:

    https://github.com/apache/accumulo/pull/82#issuecomment-202892585
  
    +1


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57000272
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, getUniqKVCount(memKeys1).size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, getUniqKVCount(memKeys2).size());
    +
    +  }
    +
    +  private int countKVPairs(List<Mutation> mutations) {
    +    int count = 0;
    +    for (Mutation m : mutations) {
    +      count += m.size();
    +    }
    +    return count;
    +  }
    +
    +  private List<MemKey> getArrayOfMemKeys(InMemoryMap imm) {
    +    SortedKeyValueIterator<Key,Value> skvi = imm.compactionIterator();
    +
    +    List<MemKey> memKeys = new ArrayList<MemKey>();
    +    try {
    +      skvi.seek(new Range(), new ArrayList<ByteSequence>(), false); // everything
    +      while (skvi.hasTop()) {
    +        memKeys.add((MemKey) skvi.getTopKey());
    +        skvi.next();
    +      }
    +    } catch (IOException ex) {
    +      java.util.logging.Logger.getLogger(InMemoryMapIT.class.getName()).log(Level.SEVERE, null, ex);
    +    }
    +
    +    return memKeys;
    +  }
    +
    +  private String dumpInMemoryMap(InMemoryMap map, List<MemKey> memkeys) {
    +    StringBuilder sb = new StringBuilder();
    +    sb.append("InMemoryMap type ");
    +    sb.append(map.getMapType());
    +    sb.append("\n");
    +
    +    for (MemKey mk : memkeys) {
    +      sb.append("  ");
    +      sb.append(mk.toString());
    +      sb.append("\n");
    +    }
    +
    +    return sb.toString();
    +  }
    +
    +  private List getUniqKVCount(List<MemKey> memKeys) {
    +    List<Integer> kvCounts = new ArrayList<Integer>();
    --- End diff --
    
    why not int?


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57575112
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,319 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    --- End diff --
    
    The Maven reactor should be smart enough to pick up artifacts from a previous module in the same execution. The only time this wouldn't work is if you `cd test` first, but it doesn't look like you're actually doing this (or, your command would work just fine from the root dir of the project).


---
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] accumulo pull request: 4148 inmemorymap counter

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

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


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56999000
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, getUniqKVCount(memKeys1).size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, getUniqKVCount(memKeys2).size());
    +
    +  }
    +
    +  private int countKVPairs(List<Mutation> mutations) {
    +    int count = 0;
    +    for (Mutation m : mutations) {
    +      count += m.size();
    +    }
    +    return count;
    +  }
    +
    +  private List<MemKey> getArrayOfMemKeys(InMemoryMap imm) {
    +    SortedKeyValueIterator<Key,Value> skvi = imm.compactionIterator();
    +
    +    List<MemKey> memKeys = new ArrayList<MemKey>();
    +    try {
    +      skvi.seek(new Range(), new ArrayList<ByteSequence>(), false); // everything
    +      while (skvi.hasTop()) {
    +        memKeys.add((MemKey) skvi.getTopKey());
    +        skvi.next();
    +      }
    +    } catch (IOException ex) {
    +      java.util.logging.Logger.getLogger(InMemoryMapIT.class.getName()).log(Level.SEVERE, null, ex);
    +    }
    +
    +    return memKeys;
    +  }
    +
    +  private String dumpInMemoryMap(InMemoryMap map, List<MemKey> memkeys) {
    +    StringBuilder sb = new StringBuilder();
    +    sb.append("InMemoryMap type ");
    +    sb.append(map.getMapType());
    +    sb.append("\n");
    +
    +    for (MemKey mk : memkeys) {
    +      sb.append("  ");
    +      sb.append(mk.toString());
    +      sb.append("\n");
    +    }
    +
    +    return sb.toString();
    +  }
    +
    +  private List getUniqKVCount(List<MemKey> memKeys) {
    +    List<Integer> kvCounts = new ArrayList<Integer>();
    --- End diff --
    
    Nevermind, i see the set at the end.  Why not just return a count instead of a list, thats what the name implies?  Returning the list was misleading for me, because the type does not feel unique.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57000165
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    --- End diff --
    
    I think it will always be true.  I initially did it this way to give a very specific message if something was wrong.  But you are correct, if the InMemoryMaps differ in size, one of the two prior asserts will fail first.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57574973
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,319 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    --- End diff --
    
    Have we already made the conscious decision that we're expecting a C++ compiler to be present for anyone doing the build (and thus would be able to build the shared library)? If we haven't, we should just JUnit's `Assume` to just ignore this test when the native IMM isnt' present.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56996932
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, getUniqKVCount(memKeys1).size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, getUniqKVCount(memKeys2).size());
    +
    +  }
    +
    +  private int countKVPairs(List<Mutation> mutations) {
    +    int count = 0;
    +    for (Mutation m : mutations) {
    +      count += m.size();
    +    }
    +    return count;
    +  }
    +
    +  private List<MemKey> getArrayOfMemKeys(InMemoryMap imm) {
    +    SortedKeyValueIterator<Key,Value> skvi = imm.compactionIterator();
    +
    +    List<MemKey> memKeys = new ArrayList<MemKey>();
    +    try {
    +      skvi.seek(new Range(), new ArrayList<ByteSequence>(), false); // everything
    +      while (skvi.hasTop()) {
    +        memKeys.add((MemKey) skvi.getTopKey());
    +        skvi.next();
    +      }
    +    } catch (IOException ex) {
    +      java.util.logging.Logger.getLogger(InMemoryMapIT.class.getName()).log(Level.SEVERE, null, ex);
    --- End diff --
    
    why just log this?  Could throw IOException up to the test methods... or wrap with RuntimeException and rethrow


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57590495
  
    --- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java ---
    @@ -202,10 +203,26 @@ public InMemoryMap(Map<String,Set<ByteSequence>> lggroups, boolean useNativeMap,
         this.memDumpDir = memDumpDir;
         this.lggroups = lggroups;
     
    -    if (lggroups.size() == 0)
    +    if (lggroups.size() == 0) {
           map = newMap(useNativeMap);
    -    else
    +      mapType = useNativeMap ? "NativeMapWrapper" : "DefaultMap";
    --- End diff --
    
    Adding constants.  Not checking them in the tests, but I could.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57590803
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,319 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    --- End diff --
    
    Ok, thanks.  And thanks for looking at this @joshelser 


---
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] accumulo pull request: 4148 inmemorymap counter

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the pull request:

    https://github.com/apache/accumulo/pull/82#issuecomment-202430768
  
    Left a few minor comments (only one real request for a change). The number of new tests is very nice. I think I'd like to make sure I can get through a full run of the test suite before I see this merged in (very hesitant around this code by nature).


---
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] accumulo pull request: 4148 inmemorymap counter

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the pull request:

    https://github.com/apache/accumulo/pull/82#issuecomment-204388925
  
    @joshelser I can help merge this forward... we can coordinate on IRC


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57590404
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,319 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    --- End diff --
    
    Ahh, I didn't read far enough to the end of the line :) to see you also provided `-pl`. I just checked what `NativeMapIT` does and this seems to be consistent, so it's probably OK as-is. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: 4148 inmemorymap counter

Posted by mjwall <gi...@git.apache.org>.
Github user mjwall commented on the pull request:

    https://github.com/apache/accumulo/pull/82#issuecomment-202183412
  
    Thanks @keith-turner, let me know if there is anything else.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56997783
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    +      nativeMapWrapper = new InMemoryMap(true, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMap = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +      localityGroupMapWithNative = new InMemoryMap(getLocalityGroups(), false, tempFolder.newFolder().getAbsolutePath());
    +    } catch (IOException e) {
    +      log.error("Error getting new InMemoryMap ", e);
    +      fail(e.getMessage());
    +    }
    +
    +    defaultMap.mutate(mutations);
    +    nativeMapWrapper.mutate(mutations);
    +    localityGroupMap.mutate(mutations);
    +    localityGroupMapWithNative.mutate(mutations);
    +
    +    // let's use the transitive property to assert all four are equivalent
    +    assertMutatesEquivalent(mutations, defaultMap, nativeMapWrapper);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMap);
    +    assertMutatesEquivalent(mutations, defaultMap, localityGroupMapWithNative);
    +  }
    +
    +  /**
    +   * Assert that a set of mutations mutate to equivalent map in both of the InMemoryMaps.
    +   * <p>
    +   * In this case, equivalent means 2 things.
    +   * <ul>
    +   * <li>The size of both maps generated is equal to the number of key value pairs in all mutations passed</li>
    +   * <li>The size of the map generated from the first InMemoryMap equals the size of the map generated from the second</li>
    +   * <li>Each key value pair in each mutated map has a unique id (kvCount)</li>
    +   * </ul>
    +   *
    +   * @param mutations
    +   *          List of mutations
    +   * @param imm1
    +   *          InMemoryMap to compare
    +   * @param imm2
    +   *          InMemoryMap to compare
    +   */
    +  private void assertMutatesEquivalent(List<Mutation> mutations, InMemoryMap imm1, InMemoryMap imm2) {
    +    int mutationKVPairs = countKVPairs(mutations);
    +
    +    List<MemKey> memKeys1 = getArrayOfMemKeys(imm1);
    +    List<MemKey> memKeys2 = getArrayOfMemKeys(imm2);
    +
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, memKeys1.size());
    +    assertEquals("Not all key value pairs included: " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, memKeys2.size());
    +    assertEquals("InMemoryMaps differ in size: " + dumpInMemoryMap(imm1, memKeys1) + "\n" + dumpInMemoryMap(imm2, memKeys2), memKeys1.size(), memKeys2.size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm1, memKeys1), mutationKVPairs, getUniqKVCount(memKeys1).size());
    +    assertEquals("InMemoryMap did not have distinct kvCounts " + dumpInMemoryMap(imm2, memKeys2), mutationKVPairs, getUniqKVCount(memKeys2).size());
    +
    +  }
    +
    +  private int countKVPairs(List<Mutation> mutations) {
    +    int count = 0;
    +    for (Mutation m : mutations) {
    +      count += m.size();
    +    }
    +    return count;
    +  }
    +
    +  private List<MemKey> getArrayOfMemKeys(InMemoryMap imm) {
    +    SortedKeyValueIterator<Key,Value> skvi = imm.compactionIterator();
    +
    +    List<MemKey> memKeys = new ArrayList<MemKey>();
    +    try {
    +      skvi.seek(new Range(), new ArrayList<ByteSequence>(), false); // everything
    +      while (skvi.hasTop()) {
    +        memKeys.add((MemKey) skvi.getTopKey());
    +        skvi.next();
    +      }
    +    } catch (IOException ex) {
    +      java.util.logging.Logger.getLogger(InMemoryMapIT.class.getName()).log(Level.SEVERE, null, ex);
    +    }
    +
    +    return memKeys;
    +  }
    +
    +  private String dumpInMemoryMap(InMemoryMap map, List<MemKey> memkeys) {
    +    StringBuilder sb = new StringBuilder();
    +    sb.append("InMemoryMap type ");
    +    sb.append(map.getMapType());
    +    sb.append("\n");
    +
    +    for (MemKey mk : memkeys) {
    +      sb.append("  ");
    +      sb.append(mk.toString());
    +      sb.append("\n");
    +    }
    +
    +    return sb.toString();
    +  }
    +
    +  private List getUniqKVCount(List<MemKey> memKeys) {
    +    List<Integer> kvCounts = new ArrayList<Integer>();
    --- End diff --
    
    Was this intended to be a `Set<Integer>`?


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57588655
  
    --- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java ---
    @@ -519,36 +520,25 @@ protected void finalize() throws Throwable {
         }
       }
     
    -  private void _mutate(Mutation mutation, int mutationCount) {
    +  private int _mutate(Mutation mutation, int mutationCount) {
     
         List<ColumnUpdate> updates = mutation.getUpdates();
         if (updates.size() == 1) {
           ColumnUpdate update = updates.get(0);
           singleUpdate(nmPointer, mutation.getRow(), update.getColumnFamily(), update.getColumnQualifier(), update.getColumnVisibility(), update.getTimestamp(),
    -          update.isDeleted(), update.getValue(), mutationCount);
    +          update.isDeleted(), update.getValue(), mutationCount++);
         } else if (updates.size() > 1) {
           long uid = startUpdate(nmPointer, mutation.getRow());
           for (ColumnUpdate update : updates) {
             update(nmPointer, uid, update.getColumnFamily(), update.getColumnQualifier(), update.getColumnVisibility(), update.getTimestamp(), update.isDeleted(),
    -            update.getValue(), mutationCount);
    +            update.getValue(), mutationCount++);
           }
    -
         }
    +    return mutationCount;
       }
     
       public void mutate(Mutation mutation, int mutationCount) {
    -    wlock.lock();
    --- End diff --
    
    I made mutate(Mutation, int) simple wrap the mutation in a Collections.singletonList and call the mutate(List<Mutation>, int) method.  They were slightly different code paths before.  There is a lock in the mutate(List,int) method, so it was not needing the mutate(Mutation,int) method any longer.


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r56998062
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,320 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.logging.Level;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    +      fail("Missing the native library from " + nativeMapLocation.getAbsolutePath() + "\nYou need to build the libaccumulo binary first. "
    +          + "\nTry running 'mvn clean install -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip'");
    +      // afterwards, you can run the following
    +      // mvn clean verify -Dit.test=InMemoryMapIT -Dtest=foo -DfailIfNoTests=false -Dfindbugs.skip -Dcheckstyle.skip -pl :accumulo-test
    +    }
    +    log.debug("Native map loaded");
    +
    +  }
    +
    +  @Test
    +  public void testOneMutationOneKey() {
    +    Mutation m = new Mutation("a");
    +    m.put(new Text("1cf"), new Text("1cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManyKeys() throws IOException {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m.put(new Text("2cf" + i), new Text("2cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testOneMutationManySameKeys() {
    +    Mutation m = new Mutation("a");
    +    for (int i = 1; i <= 5; i++) {
    +      // same keys
    +      m.put(new Text("3cf"), new Text("3cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(m);
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("b");
    +    m2.put(new Text("4cf"), new Text("4cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsSameOneKey() {
    +    Mutation m1 = new Mutation("a");
    +    m1.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +    Mutation m2 = new Mutation("a");
    +    m2.put(new Text("5cf"), new Text("5cq"), new Value("vala".getBytes()));
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMutlipleMutationsMultipleKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 6; i++) {
    +      m1.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("6cf" + i), new Text("6cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleSameKeys() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("7cf"), new Text("7cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2));
    +  }
    +
    +  @Test
    +  public void testMultipleMutationsMultipleKeysSomeSame() {
    +    Mutation m1 = new Mutation("a");
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 3; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 2; i++) {
    +      m1.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m2 = new Mutation("a");
    +    for (int i = 1; i < 3; i++) {
    +      m2.put(new Text("8cf"), new Text("8cq"), new Value(Integer.toString(i).getBytes()));
    +    }
    +    for (int i = 1; i < 4; i++) {
    +      m2.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +    Mutation m3 = new Mutation("b");
    +    for (int i = 1; i < 3; i++) {
    +      m3.put(new Text("8cf" + i), new Text("8cq" + i), new Value(Integer.toString(i).getBytes()));
    +    }
    +
    +    assertEquivalentMutate(Arrays.asList(m1, m2, m3));
    +  }
    +
    +  private void assertEquivalentMutate(Mutation m) {
    +    assertEquivalentMutate(Collections.singletonList(m));
    +  }
    +
    +  private void assertEquivalentMutate(List<Mutation> mutations) {
    +    InMemoryMap defaultMap = null;
    +    InMemoryMap nativeMapWrapper = null;
    +    InMemoryMap localityGroupMap = null;
    +    InMemoryMap localityGroupMapWithNative = null;
    +
    +    try {
    +      defaultMap = new InMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
    --- End diff --
    
    I like this comprehensive test of all the different types


---
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] accumulo pull request: 4148 inmemorymap counter

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

    https://github.com/apache/accumulo/pull/82#discussion_r57590440
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/InMemoryMapIT.java ---
    @@ -0,0 +1,319 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.accumulo.test;
    +
    +import com.google.common.collect.ImmutableSet;
    +import java.io.File;
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.accumulo.core.data.ArrayByteSequence;
    +import org.apache.accumulo.core.data.ByteSequence;
    +import org.apache.accumulo.core.data.Key;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.data.Range;
    +import org.apache.accumulo.core.data.Value;
    +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
    +import org.apache.accumulo.test.functional.NativeMapIT;
    +import org.apache.accumulo.tserver.InMemoryMap;
    +import org.apache.accumulo.tserver.MemKey;
    +import org.apache.accumulo.tserver.NativeMap;
    +import org.apache.hadoop.io.Text;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.fail;
    +import org.junit.BeforeClass;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Integration Test for https://issues.apache.org/jira/browse/ACCUMULO-4148
    + * <p>
    + * User had problem writing one Mutation with multiple KV pairs that had the same key. Doing so should write out all pairs in all mutations with a unique id. In
    + * typical operation, you would only see the last one when scanning. User had a combiner on the table, and they noticed that when using InMemoryMap with
    + * NativeMapWrapper, only the last KV pair was ever written. When InMemoryMap used DefaultMap, all KV pairs were added and the behavior worked as expected.
    + *
    + * This IT inserts a variety of Mutations with and without the same KV pairs and then inspects result of InMemoryMap mutate, looking for unique id stored with
    + * each key. This unique id, shown as mc= in the MemKey toString, was originally used for scan Isolation. Writing the same key multiple times in the same
    + * mutation is a secondary use case, discussed in https://issues.apache.org/jira/browse/ACCUMULO-227. In addition to NativeMapWrapper and DefaultMap,
    + * LocalityGroupMap was add in https://issues.apache.org/jira/browse/ACCUMULO-112.
    + *
    + * This test has to be an IT in accumulo-test, because libaccumulo is built in 'integration-test' phase of accumulo-native, which currently runs right before
    + * accumulo-test. The tests for DefaultMap could move to a unit test in tserver, but they are here for convenience of viewing both at the same time.
    + */
    +public class InMemoryMapIT {
    +
    +  private static final Logger log = LoggerFactory.getLogger(InMemoryMapIT.class);
    +
    +  @Rule
    +  public TemporaryFolder tempFolder = new TemporaryFolder(new File(System.getProperty("user.dir") + "/target"));
    +
    +  @BeforeClass
    +  public static void ensureNativeLibrary() throws FileNotFoundException {
    +    File nativeMapLocation = NativeMapIT.nativeMapLocation();
    +    log.debug("Native map location " + nativeMapLocation);
    +    NativeMap.loadNativeLib(Collections.singletonList(nativeMapLocation));
    +    if (!NativeMap.isLoaded()) {
    --- End diff --
    
    So it would be preferred to ignore this IT if the libaccumulo library is not built instead of failing?  I put this check in because the tests would pass even if you didn't have libaccumulo.  The behavior of the InMemoryMap is to use non native maps and continue.


---
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.
---