From 0ced5ecb4b3b5fc49d09db4875701cd2edab50b2 Mon Sep 17 00:00:00 2001 From: zhangyi51 Date: Thu, 13 Jun 2019 16:08:49 +0800 Subject: [PATCH 1/2] fixed repeat lock in LocksTable fixed: #564 Change-Id: I026e4a6739c7f526c421cbb5c90b48e5ff8de066 --- .../com/baidu/hugegraph/util/LockUtil.java | 2 + .../baidu/hugegraph/unit/UnitTestSuite.java | 2 + .../hugegraph/unit/core/LocksTableTest.java | 237 ++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/util/LockUtil.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/util/LockUtil.java index 25dfdd4307..2c2e0d7677 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/util/LockUtil.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/util/LockUtil.java @@ -247,11 +247,13 @@ public void lockReads(String group, Collection locks) { } } this.locks.lockReads(group, newLocks); + locked.addAll(newLocks); } // NOTE: when used in multi-threads, should add `synchronized` public void unlock() { this.locks.unlock(); + this.table.clear(); } private Set locksOfGroup(String group) { diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/UnitTestSuite.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/UnitTestSuite.java index 3780dc15cf..72ca47f866 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/UnitTestSuite.java @@ -34,6 +34,7 @@ import com.baidu.hugegraph.unit.core.DirectionsTest; import com.baidu.hugegraph.unit.core.EdgeIdTest; import com.baidu.hugegraph.unit.core.JsonUtilTest; +import com.baidu.hugegraph.unit.core.LocksTableTest; import com.baidu.hugegraph.unit.core.SerialEnumTest; import com.baidu.hugegraph.unit.core.StringUtilTest; import com.baidu.hugegraph.unit.core.VersionTest; @@ -58,6 +59,7 @@ AnalyzerTest.class, JsonUtilTest.class, StringUtilTest.class, + LocksTableTest.class, RocksDBSessionsTest.class, RocksDBCountersTest.class diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java new file mode 100644 index 0000000000..e8666fcb44 --- /dev/null +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java @@ -0,0 +1,237 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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 com.baidu.hugegraph.unit.core; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.locks.Lock; + +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.baidu.hugegraph.backend.id.Id; +import com.baidu.hugegraph.backend.id.IdGenerator; +import com.baidu.hugegraph.concurrent.LockManager; +import com.baidu.hugegraph.testutil.Assert; +import com.baidu.hugegraph.testutil.Whitebox; +import com.baidu.hugegraph.unit.BaseUnitTest; +import com.baidu.hugegraph.util.LockUtil; +import com.google.common.collect.ImmutableSet; + +public class LocksTableTest extends BaseUnitTest { + + private LockUtil.LocksTable locksTable; + private static String GRAPH = "graph"; + + @BeforeClass + public static void setup() { + LockManager.instance().create(GRAPH + "_" + "group"); + LockManager.instance().create(GRAPH + "_" + "group1"); + } + + @Before + public void initLockTable() { + this.locksTable = new LockUtil.LocksTable(GRAPH); + } + + @AfterClass + public static void teardown() { + LockManager.instance().destroy(GRAPH + "_" + "group"); + LockManager.instance().destroy(GRAPH + "_" + "group1"); + } + + @Test + public void testLocksTable() { + this.locksTable.lockReads("group", IdGenerator.of(1)); + LockUtil.Locks locks = Whitebox.getInternalState(this.locksTable, + "locks"); + Map> table = Whitebox.getInternalState(this.locksTable, + "table"); + + Assert.assertEquals(1, table.size()); + Assert.assertTrue(table.containsKey("group")); + Set ids = table.get("group"); + Assert.assertEquals(1, ids.size()); + Assert.assertTrue(ids.contains(IdGenerator.of(1))); + + List lockList = Whitebox.getInternalState(locks, "lockList"); + Assert.assertEquals(1, lockList.size()); + + this.locksTable.unlock(); + + Assert.assertEquals(0, table.size()); + Assert.assertEquals(0, lockList.size()); + } + + @Test + public void testLocksTableSameGroupDifferentLocks() { + Id id1 = IdGenerator.of(1); + Id id2 = IdGenerator.of(2); + Id id3 = IdGenerator.of(3); + Id id4 = IdGenerator.of(4); + this.locksTable.lockReads("group", id1); + this.locksTable.lockReads("group", id2); + this.locksTable.lockReads("group", id3); + this.locksTable.lockReads("group", id4); + this.locksTable.lockReads("group", id1); + this.locksTable.lockReads("group", id3); + this.locksTable.lockReads("group", id1); + LockUtil.Locks locks = Whitebox.getInternalState(this.locksTable, + "locks"); + Map> table = Whitebox.getInternalState(this.locksTable, + "table"); + + Assert.assertEquals(1, table.size()); + Assert.assertTrue(table.containsKey("group")); + Set ids = table.get("group"); + Assert.assertEquals(4, ids.size()); + Set expect = ImmutableSet.of(id1, id2, id3, id4); + Assert.assertEquals(expect, ids); + + List lockList = Whitebox.getInternalState(locks, "lockList"); + Assert.assertEquals(4, lockList.size()); + + this.locksTable.unlock(); + + Assert.assertEquals(0, table.size()); + Assert.assertEquals(0, lockList.size()); + } + + @Test + public void testLocksTableDifferentGroupDifferentLocks() { + Id id1 = IdGenerator.of(1); + Id id2 = IdGenerator.of(2); + Id id3 = IdGenerator.of(3); + Id id4 = IdGenerator.of(4); + this.locksTable.lockReads("group", id1); + this.locksTable.lockReads("group", id2); + this.locksTable.lockReads("group", id3); + this.locksTable.lockReads("group", id4); + this.locksTable.lockReads("group1", id1); + this.locksTable.lockReads("group1", id2); + this.locksTable.lockReads("group1", id1); + LockUtil.Locks locks = Whitebox.getInternalState(this.locksTable, + "locks"); + Map> table = Whitebox.getInternalState(this.locksTable, + "table"); + + Assert.assertEquals(2, table.size()); + Assert.assertTrue(table.containsKey("group")); + Assert.assertTrue(table.containsKey("group1")); + Set ids = table.get("group"); + Assert.assertEquals(4, ids.size()); + Set expect = ImmutableSet.of(id1, id2, id3, id4); + Assert.assertEquals(expect, ids); + ids = table.get("group1"); + Assert.assertEquals(2, ids.size()); + expect = ImmutableSet.of(id1, id2); + Assert.assertEquals(expect, ids); + + List lockList = Whitebox.getInternalState(locks, "lockList"); + Assert.assertEquals(6, lockList.size()); + + this.locksTable.unlock(); + + Assert.assertEquals(0, table.size()); + Assert.assertEquals(0, lockList.size()); + } + + @Test + public void testLockTableSameLock1MillionTimes() { + // ReadLock max lock times is 65535, this test ensures that there is no + // repeated locking in LocksTable + Id id = IdGenerator.of(1); + for (int i = 0; i < 1000000; i++) { + this.locksTable.lockReads("group", id); + } + LockUtil.Locks locks = Whitebox.getInternalState(this.locksTable, + "locks"); + Map> table = Whitebox.getInternalState(this.locksTable, + "table"); + + Assert.assertEquals(1, table.size()); + Assert.assertTrue(table.containsKey("group")); + Set ids = table.get("group"); + Assert.assertEquals(1, ids.size()); + Set expect = ImmutableSet.of(id); + Assert.assertEquals(expect, ids); + + List lockList = Whitebox.getInternalState(locks, "lockList"); + Assert.assertEquals(1, lockList.size()); + + this.locksTable.unlock(); + + Assert.assertEquals(0, table.size()); + Assert.assertEquals(0, lockList.size()); + } + + @Test + public void testLockWithMultiThreads() { + runWithThreads(10, () -> { + LockUtil.LocksTable locksTable = new LockUtil.LocksTable(GRAPH); + for (int i = 0; i < 10000; i++) { + locksTable.lockReads("group", IdGenerator.of(i)); + } + LockUtil.Locks locks = Whitebox.getInternalState(locksTable, + "locks"); + Map> table = Whitebox.getInternalState(locksTable, + "table"); + + Assert.assertEquals(1, table.size()); + Assert.assertTrue(table.containsKey("group")); + Set ids = table.get("group"); + Assert.assertEquals(10000, ids.size()); + + List lockList = Whitebox.getInternalState(locks, "lockList"); + Assert.assertEquals(10000, lockList.size()); + + locksTable.unlock(); + }); + } + + @Test + public void testSameLockWithMultiThreads() { + Id id = IdGenerator.of(1); + runWithThreads(10, () -> { + LockUtil.LocksTable locksTable = new LockUtil.LocksTable(GRAPH); + for (int i = 0; i < 10000; i++) { + locksTable.lockReads("group", id); + } + LockUtil.Locks locks = Whitebox.getInternalState(locksTable, + "locks"); + Map> table = Whitebox.getInternalState(locksTable, + "table"); + + Assert.assertEquals(1, table.size()); + Assert.assertTrue(table.containsKey("group")); + Set ids = table.get("group"); + Assert.assertEquals(1, ids.size()); + Set expect = ImmutableSet.of(id); + Assert.assertEquals(expect, ids); + + List lockList = Whitebox.getInternalState(locks, "lockList"); + Assert.assertEquals(1, lockList.size()); + locksTable.unlock(); + }); + } +} From de456850788f0a9e0c2f603ab807e5c7dd1211f6 Mon Sep 17 00:00:00 2001 From: zhangyi51 Date: Fri, 14 Jun 2019 15:05:46 +0800 Subject: [PATCH 2/2] improve Change-Id: I7cb72c56a70f4a5f9cd322d67dedacc4dbcde16a --- .../hugegraph/unit/core/LocksTableTest.java | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java index e8666fcb44..83ee473895 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/LocksTableTest.java @@ -45,8 +45,8 @@ public class LocksTableTest extends BaseUnitTest { @BeforeClass public static void setup() { - LockManager.instance().create(GRAPH + "_" + "group"); - LockManager.instance().create(GRAPH + "_" + "group1"); + genLockGroup(GRAPH, "group"); + genLockGroup(GRAPH, "group1"); } @Before @@ -56,8 +56,8 @@ public void initLockTable() { @AfterClass public static void teardown() { - LockManager.instance().destroy(GRAPH + "_" + "group"); - LockManager.instance().destroy(GRAPH + "_" + "group1"); + destoryLockGroup(GRAPH, "group"); + destoryLockGroup(GRAPH, "group1"); } @Test @@ -206,6 +206,8 @@ public void testLockWithMultiThreads() { Assert.assertEquals(10000, lockList.size()); locksTable.unlock(); + Assert.assertEquals(0, table.size()); + Assert.assertEquals(0, lockList.size()); }); } @@ -232,6 +234,20 @@ public void testSameLockWithMultiThreads() { List lockList = Whitebox.getInternalState(locks, "lockList"); Assert.assertEquals(1, lockList.size()); locksTable.unlock(); + Assert.assertEquals(0, table.size()); + Assert.assertEquals(0, lockList.size()); }); } + + private static void genLockGroup(String graph, String group) { + LockManager.instance().create(groupName(graph, group)); + } + + private static void destoryLockGroup(String graph, String group) { + LockManager.instance().destroy(groupName(graph, group)); + } + + private static String groupName(String graph, String group) { + return String.join("_", graph, group); + } }