Repository: kudu Updated Branches: refs/heads/master 4dbfb6247 -> cf38b5218
[java] - Synchronize the removal of the TabletClient from client2tablets and ip2client When entries are added to the maps they are added under a synchronized block, but since entries where never removed from client2tablets the removal was never synchronized. This was causing an error under concurrency where a disconnect would remove the client from both maps, but a simultaneous master lookup would not see the removal in ip2client and thus expect that client2tablets still have the entry, which was no longer the case. This makes it so that we remove the entries from both maps under the same lock we use to add them. I've ran the test 100 times without failures. Change-Id: I86197aed50be52588c3debe590d660c709cddacd Reviewed-on: http://gerrit.cloudera.org:8080/4380 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/cf38b521 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/cf38b521 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/cf38b521 Branch: refs/heads/master Commit: cf38b52185c6592d127d2f6b44f013703bd313b2 Parents: 4dbfb62 Author: David Alves <[email protected]> Authored: Sun Sep 11 22:03:23 2016 -0700 Committer: Jean-Daniel Cryans <[email protected]> Committed: Wed Sep 14 03:49:16 2016 +0000 ---------------------------------------------------------------------- .../org/apache/kudu/client/AsyncKuduClient.java | 34 ++++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/cf38b521/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java index a4da389..c478714 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java @@ -1775,8 +1775,11 @@ public class AsyncKuduClient implements AutoCloseable { } TabletClient old; + ArrayList<RemoteTablet> tablets = null; synchronized (ip2client) { - old = ip2client.remove(hostport); + if ((old = ip2client.remove(hostport)) != null) { + tablets = client2tablets.remove(client); + } } LOG.debug("Removed from IP cache: {" + hostport + "} -> {" + client + "}"); if (old == null) { @@ -1788,28 +1791,17 @@ public class AsyncKuduClient implements AutoCloseable { + hostport + "), it was found that there was no entry" + " corresponding to " + remote + ". This shouldn't happen."); } else { - removeClient(old); - } - } - - /** - * Remove all references to a client from client2tablets and from the remoteTablets - * that reference it. - * @param client tablet client to remove - */ - void removeClient(final TabletClient client) { - ArrayList<RemoteTablet> tablets = client2tablets.get(client); - if (tablets != null) { - // Make a copy so we don't need to synchronize on it while iterating. - RemoteTablet[] tablets_copy; - synchronized (tablets) { - tablets_copy = tablets.toArray(new RemoteTablet[tablets.size()]); - } - for (final RemoteTablet remoteTablet : tablets_copy) { - remoteTablet.removeTabletClient(client); + if (tablets != null) { + // Make a copy so we don't need to synchronize on it while iterating. + RemoteTablet[] tablets_copy; + synchronized (tablets) { + tablets_copy = tablets.toArray(new RemoteTablet[tablets.size()]); + } + for (final RemoteTablet remoteTablet : tablets_copy) { + remoteTablet.removeTabletClient(client); + } } } - client2tablets.remove(client); } /**
