brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aleds...@apache.org
Subject [1/6] git commit: PR #81: incorporate code review comments
Date Tue, 05 Aug 2014 20:43:51 GMT
Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master 367925e3a -> 371a351fa


PR #81: incorporate code review comments

- fix logging in CassandraDatacenterImpl
- fix CassandraNodeImpl.getTokens
- AstyanaxSupport: make keyspace name configurable, and better
  handle writeData failures when retries enabled.
- fix CassandraDatacenterLiveTest for v2: don’t explicitly set
  the JAVA_HOME
- add javadoc to MathPredicates.


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/69906235
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/69906235
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/69906235

Branch: refs/heads/master
Commit: 6990623530ffa5bace0c484ff668e2530542b9be
Parents: 683bbbd
Author: Aled Sage <aled.sage@gmail.com>
Authored: Tue Jul 29 12:11:25 2014 +0100
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Tue Aug 5 16:08:18 2014 +0100

----------------------------------------------------------------------
 .../cassandra/CassandraDatacenterImpl.java      |  8 ++---
 .../nosql/cassandra/CassandraNodeImpl.java      |  6 ++--
 .../entity/nosql/cassandra/AstyanaxSupport.java | 36 +++++++++++++-------
 .../cassandra/CassandraDatacenterLiveTest.java  | 14 +++++---
 .../java/brooklyn/util/math/MathPredicates.java | 16 +++++++++
 5 files changed, 58 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/69906235/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterImpl.java
----------------------------------------------------------------------
diff --git a/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterImpl.java
b/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterImpl.java
index cf160b1..9bf442b 100644
--- a/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterImpl.java
+++ b/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterImpl.java
@@ -113,22 +113,22 @@ public class CassandraDatacenterImpl extends DynamicClusterImpl implements
Cassa
                     Set<Entity> currentSeeds = getAttribute(CURRENT_SEEDS);
                     if (getAttribute(SERVICE_STATE) == Lifecycle.STARTING) {
                         if (Sets.intersection(currentSeeds, potentialSeeds).isEmpty()) {
-                            log.warn("Cluster {} lost all its seeds while starting! Subsequent
failure likely, but changing seeds during startup would risk split-brain: seeds={}", new Object[]
{this, currentSeeds});
+                            log.warn("Cluster {} lost all its seeds while starting! Subsequent
failure likely, but changing seeds during startup would risk split-brain: seeds={}", new Object[]
{CassandraDatacenterImpl.this, currentSeeds});
                         }
                         return currentSeeds;
                     } else if (potentialRunningSeeds.isEmpty()) {
                         // TODO Could be race where nodes have only just returned from start()
and are about to 
                         // transition to serviceUp; so don't just abandon all our seeds!
-                        log.warn("Cluster {} has no running seeds (yet?); leaving seeds as-is;
but risks split-brain if these seeds come back up!", new Object[] {this});
+                        log.warn("Cluster {} has no running seeds (yet?); leaving seeds as-is;
but risks split-brain if these seeds come back up!", new Object[] {CassandraDatacenterImpl.this});
                         return currentSeeds;
                     } else {
                         Set<Entity> result = trim(quorumSize, potentialRunningSeeds);
-                        log.debug("Cluster {} updating seeds: chosen={}; potentialRunning={}",
new Object[] {this, result, potentialRunningSeeds});
+                        log.debug("Cluster {} updating seeds: chosen={}; potentialRunning={}",
new Object[] {CassandraDatacenterImpl.this, result, potentialRunningSeeds});
                         return result;
                     }
                 } else {
                     Set<Entity> result = trim(quorumSize, potentialSeeds);
-                    if (log.isDebugEnabled()) log.debug("Cluster {} has reached seed quorum:
seeds={}", new Object[] {this, result});
+                    if (log.isDebugEnabled()) log.debug("Cluster {} has reached seed quorum:
seeds={}", new Object[] {CassandraDatacenterImpl.this, result});
                     return result;
                 }
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/69906235/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraNodeImpl.java
----------------------------------------------------------------------
diff --git a/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraNodeImpl.java
b/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraNodeImpl.java
index e8ff089..3056495 100644
--- a/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraNodeImpl.java
+++ b/software/nosql/src/main/java/brooklyn/entity/nosql/cassandra/CassandraNodeImpl.java
@@ -199,6 +199,8 @@ public class CassandraNodeImpl extends SoftwareProcessImpl implements
CassandraN
     }
     
     @Override public Set<BigInteger> getTokens() {
+        // Prefer an already-set attribute over the config.
+        // Prefer TOKENS over TOKEN.
         Set<BigInteger> tokens = getAttribute(CassandraNode.TOKENS);
         if (tokens == null) {
             BigInteger token = getAttribute(CassandraNode.TOKEN);
@@ -207,10 +209,10 @@ public class CassandraNodeImpl extends SoftwareProcessImpl implements
CassandraN
             }
         }
         if (tokens == null) {
-            tokens = getAttribute(CassandraNode.TOKENS);
+            tokens = getConfig(CassandraNode.TOKENS);
         }
         if (tokens == null) {
-            BigInteger token = getAttribute(CassandraNode.TOKEN);
+            BigInteger token = getConfig(CassandraNode.TOKEN);
             if (token != null) {
                 tokens = ImmutableSet.of(token);
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/69906235/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/AstyanaxSupport.java
----------------------------------------------------------------------
diff --git a/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/AstyanaxSupport.java
b/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/AstyanaxSupport.java
index 7fdafc8..5395705 100644
--- a/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/AstyanaxSupport.java
+++ b/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/AstyanaxSupport.java
@@ -166,17 +166,18 @@ public class AstyanaxSupport {
          * Exercise the {@link CassandraNode} using the Astyanax API.
          */
         public void astyanaxTest() throws Exception {
-            writeData();
-            readData();
+            String keyspaceName = "BrooklynTests_"+Identifiers.makeRandomId(8);
+            writeData(keyspaceName);
+            readData(keyspaceName);
         }
 
         /**
          * Write to a {@link CassandraNode} using the Astyanax API.
          * @throws ConnectionException 
          */
-        public void writeData() throws ConnectionException {
+        public void writeData(String keyspaceName) throws ConnectionException {
             // Create context
-            AstyanaxContext<Keyspace> context = newAstyanaxContextForKeyspace("BrooklynIntegrationTest");
+            AstyanaxContext<Keyspace> context = newAstyanaxContextForKeyspace(keyspaceName);
             try {
                 Keyspace keyspace = context.getEntity();
                 try {
@@ -233,14 +234,14 @@ public class AstyanaxSupport {
                 context.shutdown();
             }
         }
-        
+
         /**
          * Read from a {@link CassandraNode} using the Astyanax API.
          * @throws ConnectionException 
          */
-        public void readData() throws ConnectionException {
+        public void readData(String keyspaceName) throws ConnectionException {
             // Create context
-            AstyanaxContext<Keyspace> context = newAstyanaxContextForKeyspace("BrooklynIntegrationTest");
+            AstyanaxContext<Keyspace> context = newAstyanaxContextForKeyspace(keyspaceName);
             try {
                 Keyspace keyspace = context.getEntity();
 
@@ -268,12 +269,19 @@ public class AstyanaxSupport {
         }
         
 
-        public void writeData(int numRetries) throws ConnectionException {
+        /**
+         * Returns the keyspace name to which the data has been written. If it fails the
first time,
+         * then will increment the keyspace name. This is because the failure could be a
response timeout,
+         * where the keyspace really has been created so subsequent attempts with the same
name will 
+         * fail (because we assert that the keyspace did not exist).
+         */
+        public String writeData(String keyspacePrefix, int numRetries) throws ConnectionException
{
             int retryCount = 0;
             while (true) {
                 try {
-                    writeData();
-                    return;
+                    String keyspaceName = keyspacePrefix + (retryCount > 0 ? "" : "_"+retryCount);
+                    writeData(keyspaceName);
+                    return keyspaceName;
                 } catch (Exception e) {
                     log.warn("Error writing data - attempt "+(retryCount+1)+" of "+(numRetries+1)+":
"+e, e);
                     if (++retryCount > numRetries)
@@ -282,11 +290,15 @@ public class AstyanaxSupport {
             }
         }
 
-        public void readData(int numRetries) throws ConnectionException {
+        /**
+         * Repeatedly tries to read data from the given keyspace name. Asserts that the data
is the
+         * same as would be written by calling {@code writeData(keyspaceName)}.
+         */
+        public void readData(String keyspaceName, int numRetries) throws ConnectionException
{
             int retryCount = 0;
             while (true) {
                 try {
-                    readData();
+                    readData(keyspaceName);
                     return;
                 } catch (Exception e) {
                     log.warn("Error reading data - attempt "+(retryCount+1)+" of "+(numRetries+1)+":
"+e, e);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/69906235/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterLiveTest.java
----------------------------------------------------------------------
diff --git a/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterLiveTest.java
b/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterLiveTest.java
index 44c27b7..809a738 100644
--- a/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterLiveTest.java
+++ b/software/nosql/src/test/java/brooklyn/entity/nosql/cassandra/CassandraDatacenterLiveTest.java
@@ -110,17 +110,22 @@ public class CassandraDatacenterLiveTest extends BrooklynAppLiveTestSupport
{
         runCluster(spec, true);
     }
     
+    /*
+     * TODO on some distros (e.g. CentOS?), it comes pre-installed with java 6. Installing
java 7 
+     * didn't seem to be enough. I also had to set JAVA_HOME:
+     *     .configure("shell.env", MutableMap.of("JAVA_HOME", "/etc/alternatives/java_sdk_1.7.0"))
+     * However, that would break other deployments such as on Ubuntu where JAVA_HOME would
be different.
+     */
     @Test(groups = "Live")
     public void testDatacenterWithVnodesVersion2() throws Exception {
         EntitySpec<CassandraDatacenter> spec = EntitySpec.create(CassandraDatacenter.class)
                 .configure("initialSize", 2)
                 .configure(CassandraNode.SUGGESTED_VERSION, "2.0.9")
-                .configure("shell.env", MutableMap.of("JAVA_HOME", "/etc/alternatives/java_sdk_1.7.0"))
                 .configure(CassandraDatacenter.USE_VNODES, true)
                 .configure("clusterName", "CassandraClusterLiveTest");
         runCluster(spec, true);
     }
-    
+
     @Test(groups = {"Live", "Acceptance"}, invocationCount=10)
     public void testManyTimes() throws Exception {
         testDatacenter();
@@ -268,12 +273,13 @@ public class CassandraDatacenterLiveTest extends BrooklynAppLiveTestSupport
{
         if (versions.size() > 1) {
             Assert.fail("Inconsistent versions on Cassandra start: "+versions);
         }
+        String keyspacePrefix = "BrooklynTests_"+Identifiers.makeRandomId(8);
 
-        astyanaxFirst.writeData(numRetries);
+        String keyspaceName = astyanaxFirst.writeData(keyspacePrefix, numRetries);
 
         for (Entity node : nodes) {
             AstyanaxSample astyanaxSecond = AstyanaxSample.builder().node((CassandraNode)node).columnFamilyName(uniqueName).build();
-            astyanaxSecond.readData(numRetries);
+            astyanaxSecond.readData(keyspaceName, numRetries);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/69906235/utils/common/src/main/java/brooklyn/util/math/MathPredicates.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/math/MathPredicates.java b/utils/common/src/main/java/brooklyn/util/math/MathPredicates.java
index 1f2f724..911f308 100644
--- a/utils/common/src/main/java/brooklyn/util/math/MathPredicates.java
+++ b/utils/common/src/main/java/brooklyn/util/math/MathPredicates.java
@@ -24,6 +24,10 @@ import com.google.common.base.Predicate;
 
 public class MathPredicates {
 
+    /**
+     * Creates a predicate comparing a given number with {@code val}. 
+     * A number of {@code null} passed to the predicate will always return false.
+     */
     public static Predicate<Number> greaterThan(final double val) {
         return new Predicate<Number>() {
             public boolean apply(@Nullable Number input) {
@@ -32,6 +36,10 @@ public class MathPredicates {
         };
     }
 
+    /**
+     * Creates a predicate comparing a given number with {@code val}. 
+     * A number of {@code null} passed to the predicate will always return false.
+     */
     public static Predicate<Number> greaterThanOrEqual(final double val) {
         return new Predicate<Number>() {
             public boolean apply(@Nullable Number input) {
@@ -40,6 +48,10 @@ public class MathPredicates {
         };
     }
 
+    /**
+     * Creates a predicate comparing a given number with {@code val}. 
+     * A number of {@code null} passed to the predicate will always return false.
+     */
     public static Predicate<Number> lessThan(final double val) {
         return new Predicate<Number>() {
             public boolean apply(@Nullable Number input) {
@@ -48,6 +60,10 @@ public class MathPredicates {
         };
     }
 
+    /**
+     * Creates a predicate comparing a given number with {@code val}. 
+     * A number of {@code null} passed to the predicate will always return false.
+     */
     public static Predicate<Number> lessThanOrEqual(final double val) {
         return new Predicate<Number>() {
             public boolean apply(@Nullable Number input) {


Mime
View raw message