Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 82D2D10BC4 for ; Tue, 17 Dec 2013 14:50:00 +0000 (UTC) Received: (qmail 82779 invoked by uid 500); 17 Dec 2013 14:49:58 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 82599 invoked by uid 500); 17 Dec 2013 14:49:56 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 82080 invoked by uid 99); 17 Dec 2013 14:49:55 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 17 Dec 2013 14:49:55 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 9910E8BA0EC; Tue, 17 Dec 2013 14:49:55 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jbellis@apache.org To: commits@cassandra.apache.org Date: Tue, 17 Dec 2013 14:49:59 -0000 Message-Id: In-Reply-To: <095d3dbcb4de40a88af75974f9d55545@git.apache.org> References: <095d3dbcb4de40a88af75974f9d55545@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [5/8] git commit: caching all calls of cloneOnlyTokenMap is not correct since many callers mutate the result patch by Aleksey Yeschenko; reviewed by jbellis for CASSANDRA-6488 caching all calls of cloneOnlyTokenMap is not correct since many callers mutate the result patch by Aleksey Yeschenko; reviewed by jbellis for CASSANDRA-6488 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/829047af Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/829047af Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/829047af Branch: refs/heads/cassandra-1.2 Commit: 829047af58fb1735b5d12b74f06ed0d4f04b2c0d Parents: 54a1955 Author: Jonathan Ellis Authored: Tue Dec 17 08:46:35 2013 -0600 Committer: Jonathan Ellis Committed: Tue Dec 17 08:46:35 2013 -0600 ---------------------------------------------------------------------- CHANGES.txt | 6 ++--- .../locator/AbstractReplicationStrategy.java | 2 +- .../apache/cassandra/locator/TokenMetadata.java | 28 +++++++++++++++++--- .../apache/cassandra/service/StorageProxy.java | 2 +- 4 files changed, 29 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/829047af/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 4816d70..22a121e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,9 +1,8 @@ -1.2.14 - * Improved error message on bad properties in DDL queries (CASSANDRA-6453) - 1.2.13 + * Improved error message on bad properties in DDL queries (CASSANDRA-6453) * Randomize batchlog candidates selection (CASSANDRA-6481) * Fix thundering herd on endpoint cache invalidation (CASSANDRA-6345, 6485) + * Improve batchlog write performance with vnodes (CASSANDRA-6488) * Optimize FD phi calculation (CASSANDRA-6386) * Improve initial FD phi estimate when starting up (CASSANDRA-6385) * Don't list CQL3 table in CLI describe even if named explicitely @@ -20,7 +19,6 @@ (CASSANDRA-6413) * (Hadoop) add describe_local_ring (CASSANDRA-6268) * Fix handling of concurrent directory creation failure (CASSANDRA-6459) - * Improve batchlog write performance with vnodes (CASSANDRA-6488) 1.2.12 http://git-wip-us.apache.org/repos/asf/cassandra/blob/829047af/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java index 85e229c..a48bec9 100644 --- a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java +++ b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java @@ -107,7 +107,7 @@ public abstract class AbstractReplicationStrategy ArrayList endpoints = getCachedEndpoints(keyToken); if (endpoints == null) { - TokenMetadata tm = tokenMetadata.cloneOnlyTokenMap(); + TokenMetadata tm = tokenMetadata.cachedOnlyTokenMap(); // if our cache got invalidated, it's possible there is a new token to account for too keyToken = TokenMetadata.firstToken(tm.sortedTokens(), searchToken); endpoints = new ArrayList(calculateNaturalEndpoints(searchToken, tm)); http://git-wip-us.apache.org/repos/asf/cassandra/blob/829047af/src/java/org/apache/cassandra/locator/TokenMetadata.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/locator/TokenMetadata.java b/src/java/org/apache/cassandra/locator/TokenMetadata.java index be0f7c7..cf0c472 100644 --- a/src/java/org/apache/cassandra/locator/TokenMetadata.java +++ b/src/java/org/apache/cassandra/locator/TokenMetadata.java @@ -591,12 +591,31 @@ public class TokenMetadata /** * Create a copy of TokenMetadata with only tokenToEndpointMap. That is, pending ranges, * bootstrap tokens and leaving endpoints are not included in the copy. - * - * This uses a cached copy that is invalided when the ring changes, so in the common case - * no extra locking is required. */ public TokenMetadata cloneOnlyTokenMap() { + lock.readLock().lock(); + try + { + return new TokenMetadata(SortedBiMultiValMap.create(tokenToEndpointMap, null, inetaddressCmp), + HashBiMap.create(endpointToHostIdMap), + new Topology(topology)); + } + finally + { + lock.readLock().unlock(); + } + } + + /** + * Return a cached TokenMetadata with only tokenToEndpointMap, i.e., the same as cloneOnlyTokenMap but + * uses a cached copy that is invalided when the ring changes, so in the common case + * no extra locking is required. + * + * Callers must *NOT* mutate the returned metadata object. + */ + public TokenMetadata cachedOnlyTokenMap() + { TokenMetadata tm = cachedTokenMap.get(); if (tm != null) return tm; @@ -604,6 +623,9 @@ public class TokenMetadata // synchronize is to prevent thundering herd (CASSANDRA-6345); lock.readLock is for correctness vs updates to our internals synchronized (this) { + if ((tm = cachedTokenMap.get()) != null) + return tm; + lock.readLock().lock(); try { http://git-wip-us.apache.org/repos/asf/cassandra/blob/829047af/src/java/org/apache/cassandra/service/StorageProxy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/StorageProxy.java b/src/java/org/apache/cassandra/service/StorageProxy.java index 376edb6..d49e59d 100644 --- a/src/java/org/apache/cassandra/service/StorageProxy.java +++ b/src/java/org/apache/cassandra/service/StorageProxy.java @@ -420,7 +420,7 @@ public class StorageProxy implements StorageProxyMBean private static Collection getBatchlogEndpoints(String localDataCenter, ConsistencyLevel consistencyLevel) throws UnavailableException { - TokenMetadata.Topology topology = StorageService.instance.getTokenMetadata().cloneOnlyTokenMap().getTopology(); + TokenMetadata.Topology topology = StorageService.instance.getTokenMetadata().cachedOnlyTokenMap().getTopology(); List localEndpoints = new ArrayList(topology.getDatacenterEndpoints().get(localDataCenter)); // special case for single-node datacenters