You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/07/14 17:36:02 UTC
[06/11] git commit: Address comments in PR #61
Address comments in PR #61
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/6997575b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/6997575b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/6997575b
Branch: refs/heads/master
Commit: 6997575ba9391784fd29abc2bc724a14c510c22e
Parents: 388087d
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Fri Jul 11 10:34:38 2014 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Fri Jul 11 10:34:38 2014 +0300
----------------------------------------------------------------------
.../brooklyn/entity/group/DynamicClusterTest.java | 16 +++++++---------
utils/common/src/main/java/brooklyn/util/os/Os.java | 12 +++++++-----
.../main/java/brooklyn/util/text/Identifiers.java | 8 +++++---
.../src/test/java/brooklyn/util/os/OsTest.java | 8 ++++----
4 files changed, 23 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6997575b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
index 191ce17..ebdd5fe 100644
--- a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
+++ b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
@@ -510,22 +510,20 @@ public class DynamicClusterTest extends BrooklynAppUnitTestSupport {
cluster.start(ImmutableList.of(loc));
cluster.resize(1);
+
+ //Prevent the two entities created in the same ms
+ //so that the removal strategy can always choose the
+ //entity created next
+ Thread.sleep(1);
+
cluster.resize(2);
assertEquals(cluster.getCurrentSize(), (Integer)2);
assertEquals(ImmutableSet.copyOf(cluster.getMembers()), ImmutableSet.copyOf(creationOrder), "actual="+cluster.getMembers());
- Collection<Entity> preStopMembers = ImmutableList.copyOf(cluster.getMembers());
-
// Now stop one
cluster.resize(1);
assertEquals(cluster.getCurrentSize(), (Integer)1);
-
- //Make sure we stopped the entity with the earliest creation time
- //Could be multiple entities with identical creation time, pick any.
- Entity remainingEntity = cluster.getMembers().iterator().next();
- for(Entity entity : preStopMembers) {
- assertTrue(entity.getCreationTime() >= remainingEntity.getCreationTime());
- }
+ assertEquals(ImmutableList.copyOf(cluster.getMembers()), creationOrder.subList(0, 1));
}
@Test
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6997575b/utils/common/src/main/java/brooklyn/util/os/Os.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/os/Os.java b/utils/common/src/main/java/brooklyn/util/os/Os.java
index eab6be0..0602fdc 100644
--- a/utils/common/src/main/java/brooklyn/util/os/Os.java
+++ b/utils/common/src/main/java/brooklyn/util/os/Os.java
@@ -532,9 +532,7 @@ public class Os {
tempFile.deleteOnExit();
return tempFile;
} catch (IOException e) {
- Exceptions.propagate(e);
- //non-reachable, prevent uninitialized warning in following code
- return null;
+ throw Exceptions.propagate(e);
}
}
@@ -547,6 +545,10 @@ public class Os {
public static File newTempDir(String prefix) {
String sanitizedPrefix = (prefix==null ? "" : prefix + "-");
String tmpParent = tmp();
+
+ //With lots of stale temp dirs it is possible to have
+ //name collisions so we need to retry until a unique
+ //name is found
for (int i = 0; i < TEMP_DIR_ATTEMPTS; i++) {
String baseName = sanitizedPrefix + Identifiers.makeRandomId(4);
File tempDir = new File(tmpParent, baseName);
@@ -555,10 +557,10 @@ public class Os {
Os.deleteOnExitRecursively(tempDir);
return tempDir;
} else {
- log.warn("Attempt to create temp dir failed " + tempDir);
+ log.warn("Attempt to create temp dir failed " + tempDir + ". Either an IO error (disk full, no rights) or someone else created the folder after the !exists() check.");
}
} else {
- log.debug("Attemp to create temp dir failed, already exists " + tempDir);
+ log.debug("Attempt to create temp dir failed, already exists " + tempDir + ". With ID of length 4 it is not unusual (15% chance) to have duplicate names at the 2000 samples mark.");
}
}
throw new IllegalStateException("cannot create temporary folders in parent " + tmpParent + " after " + TEMP_DIR_ATTEMPTS + " attempts.");
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6997575b/utils/common/src/main/java/brooklyn/util/text/Identifiers.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/text/Identifiers.java b/utils/common/src/main/java/brooklyn/util/text/Identifiers.java
index 11bda41..1b8377d 100644
--- a/utils/common/src/main/java/brooklyn/util/text/Identifiers.java
+++ b/utils/common/src/main/java/brooklyn/util/text/Identifiers.java
@@ -40,6 +40,11 @@ public class Identifiers {
* tests ensure random distribution, so random ID of length 5
* is about 2^29 possibilities
* <p>
+ * With ID of length 4 it is not unlikely (15% chance) to get
+ * duplicates in the first 2000 attempts.
+ * With ID of length 8 there is 1% chance to get duplicates
+ * in the first 1M attempts and 50% for the first 16M.
+ * <p>
* implementation is efficient, uses char array, and
* makes one call to random per 5 chars; makeRandomId(5)
* takes about 4 times as long as a simple Math.random call,
@@ -51,9 +56,6 @@ public class Identifiers {
* in general this is preferable to base64 as is more portable,
* can be used throughout javascript (as ID's which don't allow +)
* or as java identifiers (which don't allow numbers in the first char)
- *
- * WARNING: The method is not as random as advertised!
- * Highly probable to return the same id in the first 2000 attempts.
**/
public static String makeRandomId(int l) {
//this version is 30-50% faster than the old double-based one,
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6997575b/utils/common/src/test/java/brooklyn/util/os/OsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/brooklyn/util/os/OsTest.java b/utils/common/src/test/java/brooklyn/util/os/OsTest.java
index 54f39af..93710f8 100644
--- a/utils/common/src/test/java/brooklyn/util/os/OsTest.java
+++ b/utils/common/src/test/java/brooklyn/util/os/OsTest.java
@@ -89,9 +89,9 @@ public class OsTest {
assertEquals(Os.mergePaths("/a", "b"), "/a/b");
}
- @Test
+ @Test(groups="Integration")
public void testNewTempFile() {
- int CREATE_CNT = 1500;
+ int CREATE_CNT = 5000;
Collection<File> folders = new ArrayList<File>(CREATE_CNT);
try {
@@ -111,9 +111,9 @@ public class OsTest {
}
}
- @Test
+ @Test(groups="Integration")
public void testNewTempDir() {
- int CREATE_CNT = 2000;
+ int CREATE_CNT = 5000;
Collection<File> folders = new ArrayList<File>(CREATE_CNT);
try {