You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by jd...@apache.org on 2016/09/14 03:49:26 UTC
kudu git commit: [java] - Synchronize the removal of the TabletClient
from client2tablets and ip2client
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 <jd...@apache.org>
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 <dr...@apache.org>
Authored: Sun Sep 11 22:03:23 2016 -0700
Committer: Jean-Daniel Cryans <jd...@apache.org>
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);
}
/**