ACCUMULO-4143 Make protective copy of migrations Makes a protective copy of the master's migrations, when requested, to prevent ConcurrentModificationExceptions when the requestor iterates over the set at the same time the master is updating it.
This closes #68 Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/f1e4f9a3 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/f1e4f9a3 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/f1e4f9a3 Branch: refs/heads/1.7 Commit: f1e4f9a3a1db62154b3b3cd199560243960d3484 Parents: bac90d9 Author: Christopher Tubbs <ctubb...@apache.org> Authored: Thu Feb 11 18:51:56 2016 -0500 Committer: Christopher Tubbs <ctubb...@apache.org> Committed: Thu Feb 11 18:51:56 2016 -0500 ---------------------------------------------------------------------- .../accumulo/server/master/state/CurrentState.java | 5 ++++- .../server/master/state/MetaDataTableScanner.java | 2 +- .../main/java/org/apache/accumulo/master/Master.java | 14 +++++++------- .../org/apache/accumulo/master/TestMergeState.java | 4 ++-- .../test/functional/TabletStateChangeIteratorIT.java | 4 ++-- 5 files changed, 16 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/f1e4f9a3/server/base/src/main/java/org/apache/accumulo/server/master/state/CurrentState.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/master/state/CurrentState.java b/server/base/src/main/java/org/apache/accumulo/server/master/state/CurrentState.java index b07a931..467c7b7 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/master/state/CurrentState.java +++ b/server/base/src/main/java/org/apache/accumulo/server/master/state/CurrentState.java @@ -29,5 +29,8 @@ public interface CurrentState { Collection<MergeInfo> merges(); - Collection<KeyExtent> migrations(); + /** + * Provide an immutable snapshot view of migrating tablets. Objects contained in the set may still be mutable. + */ + Set<KeyExtent> migrationsSnapshot(); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f1e4f9a3/server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java b/server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java index 8adce32..c6ad89b 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java +++ b/server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java @@ -86,7 +86,7 @@ public class MetaDataTableScanner implements ClosableIterator<TabletLocationStat TabletStateChangeIterator.setCurrentServers(tabletChange, state.onlineTabletServers()); TabletStateChangeIterator.setOnlineTables(tabletChange, state.onlineTables()); TabletStateChangeIterator.setMerges(tabletChange, state.merges()); - TabletStateChangeIterator.setMigrations(tabletChange, state.migrations()); + TabletStateChangeIterator.setMigrations(tabletChange, state.migrationsSnapshot()); } scanner.addScanIterator(tabletChange); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f1e4f9a3/server/master/src/main/java/org/apache/accumulo/master/Master.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/Master.java b/server/master/src/main/java/org/apache/accumulo/master/Master.java index 78d26bf..cc4a02f 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/Master.java +++ b/server/master/src/main/java/org/apache/accumulo/master/Master.java @@ -902,11 +902,7 @@ public class Master implements LiveTServerSet.Listener, TableObserver, CurrentSt private long balanceTablets() { List<TabletMigration> migrationsOut = new ArrayList<TabletMigration>(); - Set<KeyExtent> migrationsCopy = new HashSet<KeyExtent>(); - synchronized (migrations) { - migrationsCopy.addAll(migrations.keySet()); - } - long wait = tabletBalancer.balance(Collections.unmodifiableSortedMap(tserverStatus), Collections.unmodifiableSet(migrationsCopy), migrationsOut); + long wait = tabletBalancer.balance(Collections.unmodifiableSortedMap(tserverStatus), migrationsSnapshot(), migrationsOut); for (TabletMigration m : TabletBalancer.checkMigrationSanity(tserverStatus.keySet(), migrationsOut)) { if (migrations.containsKey(m.tablet)) { @@ -1336,7 +1332,11 @@ public class Master implements LiveTServerSet.Listener, TableObserver, CurrentSt } @Override - public Collection<KeyExtent> migrations() { - return migrations.keySet(); + public Set<KeyExtent> migrationsSnapshot() { + Set<KeyExtent> migrationsCopy = new HashSet<KeyExtent>(); + synchronized (migrations) { + migrationsCopy.addAll(migrations.keySet()); + } + return Collections.unmodifiableSet(migrationsCopy); } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f1e4f9a3/server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java ---------------------------------------------------------------------- diff --git a/server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b/server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java index b84df2b..045a383 100644 --- a/server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java +++ b/server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java @@ -82,8 +82,8 @@ public class TestMergeState { } @Override - public Collection<KeyExtent> migrations() { - return Collections.emptyList(); + public Set<KeyExtent> migrationsSnapshot() { + return Collections.emptySet(); } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f1e4f9a3/test/src/test/java/org/apache/accumulo/test/functional/TabletStateChangeIteratorIT.java ---------------------------------------------------------------------- diff --git a/test/src/test/java/org/apache/accumulo/test/functional/TabletStateChangeIteratorIT.java b/test/src/test/java/org/apache/accumulo/test/functional/TabletStateChangeIteratorIT.java index 4c192f7..275255c 100644 --- a/test/src/test/java/org/apache/accumulo/test/functional/TabletStateChangeIteratorIT.java +++ b/test/src/test/java/org/apache/accumulo/test/functional/TabletStateChangeIteratorIT.java @@ -172,8 +172,8 @@ public class TabletStateChangeIteratorIT extends SharedMiniClusterIT { } @Override - public Collection<KeyExtent> migrations() { - return Collections.emptyList(); + public Set<KeyExtent> migrationsSnapshot() { + return Collections.emptySet(); } }