You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2014/07/10 11:20:45 UTC
[GitHub] incubator-brooklyn pull request: Fix failing tests on Windows
GitHub user neykov opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/61
Fix failing tests on Windows
* brooklyn.util.ResourceUtilsTest.testGetResourceViaClasspathWithoutPrefix() fails on windows paths when OSGi is used
* Explicitly wait for osgi framework shutdown before deleting the cache folder so all files are unlocked
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neykov/incubator-brooklyn fix-failing-tests
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/61.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #61
----
commit 3bdba4f37c74a466b3119a32c8532a46fd99b1aa
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-07-10T09:18:32Z
Fix failing tests on Windows
* brooklyn.util.ResourceUtilsTest.testGetResourceViaClasspathWithoutPrefix() fails on windows paths when OSGi is used
* Explicitly wait for osgi framework shutdown before deleting the cache folder so all files are unlocked
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#issuecomment-48748978
Added the comment, disabled for both reasons actually.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14809037
--- Diff: utils/common/src/main/java/brooklyn/util/os/Os.java ---
@@ -544,13 +545,23 @@ public static File newTempFile(Class<?> clazz, String ext) {
/** creates a temp dir which will be deleted on exit */
public static File newTempDir(String prefix) {
- String baseName = (prefix==null ? "" : prefix + "-") + Identifiers.makeRandomId(4);
- File tempDir = new File(tmp(), baseName);
- if (tempDir.mkdir()) {
- Os.deleteOnExitRecursively(tempDir);
- return tempDir;
+ String sanitizedPrefix = (prefix==null ? "" : prefix + "-");
+ String tmpParent = tmp();
+ for (int i = 0; i < TEMP_DIR_ATTEMPTS; i++) {
+ String baseName = sanitizedPrefix + Identifiers.makeRandomId(4);
+ File tempDir = new File(tmpParent, baseName);
+ if (!tempDir.exists()) {
+ if (tempDir.mkdir()) {
+ Os.deleteOnExitRecursively(tempDir);
+ return tempDir;
+ } else {
+ log.warn("Attempt to create temp dir failed " + tempDir);
+ }
+ } else {
+ log.debug("Attemp to create temp dir failed, already exists " + tempDir);
--- End diff --
>if the creation failed (log.warn), e.g. because a directory was not writable or disk is full
There is a third option where someone else creates the folder between our !exists() & mkdir() calls, so even in this case we can't be sure that the failure is due to io error.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/61
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14806268
--- Diff: core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java ---
@@ -514,10 +514,18 @@ public void defaultRemovalStrategyShutsDownNewestFirstWhenResizing() throws Exce
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);
- assertEquals(ImmutableList.copyOf(cluster.getMembers()), creationOrder.subList(0, 1));
+
+ //Make sure we stopped the entity with the earliest creation time
--- End diff --
we stop the entity with the **latest** creation time, not the **earliest**
(the code is correct, but the comment is not)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#issuecomment-48696229
great fixes. minor comments, worth addressing, then good to merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14806843
--- Diff: utils/common/src/main/java/brooklyn/util/os/Os.java ---
@@ -544,13 +545,23 @@ public static File newTempFile(Class<?> clazz, String ext) {
/** creates a temp dir which will be deleted on exit */
public static File newTempDir(String prefix) {
- String baseName = (prefix==null ? "" : prefix + "-") + Identifiers.makeRandomId(4);
- File tempDir = new File(tmp(), baseName);
- if (tempDir.mkdir()) {
- Os.deleteOnExitRecursively(tempDir);
- return tempDir;
+ String sanitizedPrefix = (prefix==null ? "" : prefix + "-");
+ String tmpParent = tmp();
+ for (int i = 0; i < TEMP_DIR_ATTEMPTS; i++) {
+ String baseName = sanitizedPrefix + Identifiers.makeRandomId(4);
+ File tempDir = new File(tmpParent, baseName);
+ if (!tempDir.exists()) {
+ if (tempDir.mkdir()) {
+ Os.deleteOnExitRecursively(tempDir);
+ return tempDir;
+ } else {
+ log.warn("Attempt to create temp dir failed " + tempDir);
+ }
+ } else {
+ log.debug("Attemp to create temp dir failed, already exists " + tempDir);
--- End diff --
have worked through the math. it's the old "how many people in a room until there's a better than even chance two have the same birthday" problem. suggest the comment on the `debug` explain this, that with random id of length 4 it's not unusual to have duplicates at the 2000 members mark.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14806385
--- Diff: utils/common/src/main/java/brooklyn/util/os/Os.java ---
@@ -523,17 +525,16 @@ public static boolean isMicrosoftWindows() {
* either prefix or ext may be null;
* if ext is non-empty and not > 4 chars and not starting with a ., then a dot will be inserted */
public static File newTempFile(String prefix, String ext) {
- String baseName = (prefix!=null ? prefix + "-" : "") + Identifiers.makeRandomId(4) +
- (ext!=null ? ext.startsWith(".") || ext.length()>4 ? ext : "."+ext : "");
- File tempFile = new File(tmp(), baseName);
+ String sanitizedPrefix = (prefix!=null ? prefix + "-" : "");
+ String sanitizedExt = (ext!=null ? ext.startsWith(".") || ext.length()>4 ? ext : "."+ext : "");
try {
- if (tempFile.createNewFile()) {
- tempFile.deleteOnExit();
- return tempFile;
- }
- throw new IllegalStateException("cannot create temp file "+tempFile+", call returned false");
+ File tempFile = File.createTempFile(sanitizedPrefix, sanitizedExt, new File(tmp()));
+ tempFile.deleteOnExit();
+ return tempFile;
} catch (IOException e) {
- throw new IllegalStateException("cannot create temp file "+tempFile+", error: "+e, e);
+ Exceptions.propagate(e);
--- End diff --
most of us use `throw Exceptions.propagate(e)` to skip needing to include a non-reachable `return null`; the `throw` never gets run of course but it is misleading in a good way :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14808432
--- Diff: utils/common/src/main/java/brooklyn/util/text/Identifiers.java ---
@@ -51,6 +51,9 @@
* 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.
--- End diff --
The suggestion to put concrete examples of probability is great. I was considering removing the comment altogether after your comment above about the birthday paradox, but this would be very useful to add.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#issuecomment-48704886
Addressed comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14808637
--- Diff: utils/common/src/test/java/brooklyn/util/os/OsTest.java ---
@@ -84,5 +88,49 @@ public void testMergePaths() throws Exception {
assertEquals(Os.mergePaths("a", "b/"), "a/b/");
assertEquals(Os.mergePaths("/a", "b"), "/a/b");
}
+
+ @Test
+ public void testNewTempFile() {
--- End diff --
I chose the counter so that the tests finish in less than a second on my system while still failing frequently on the old implementation. Considering that there might be slower systems than mine it is a good idea to put it in integration.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14807592
--- Diff: utils/common/src/main/java/brooklyn/util/text/Identifiers.java ---
@@ -51,6 +51,9 @@
* 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.
--- End diff --
It is as random as *advertised*, but not as random as a caller might think it is, including me!
I suggest saying "with length 4" it is "not unlikely (15% chance)" -- rather than "highly probable" -- that you'll get duplicates in the first 2000 attempts.
Probably worth noting "with length 8 there is a 1% chance of duplicates at 2M and a 50% chance of duplicates at 16M", as we use length 8 when we really care about uniqueness.
Even that is much sooner than I expected however. We could use length 12 (puts us in the range of 7B before a 1% chance of duplicates, by my calcs) -- though I tend to think a better option is a pluggable unique identifier source, which we've discussed and would be quite useful in any case, also allowing ID to indicate time order when that matters or origination (in a distributed context). /cc @aledsage
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14807613
--- Diff: utils/common/src/test/java/brooklyn/util/os/OsTest.java ---
@@ -84,5 +88,49 @@ public void testMergePaths() throws Exception {
assertEquals(Os.mergePaths("a", "b/"), "a/b/");
assertEquals(Os.mergePaths("/a", "b"), "/a/b");
}
+
+ @Test
+ public void testNewTempFile() {
+ int CREATE_CNT = 1500;
+ Collection<File> folders = new ArrayList<File>(CREATE_CNT);
+
+ try {
+ for (int i = 0; i < CREATE_CNT; i++) {
+ try {
+ folders.add(Os.newTempFile(OsTest.class, "test"));
+ } catch (IllegalStateException e) {
+ log.warn("testNewTempFile failed at " + i + " iteration.");
+ Exceptions.propagate(e);
+ }
+ }
+ } finally {
+ //cleanup
+ for (File folder : folders) {
+ folder.delete();
+ }
+ }
+ }
+
+ @Test
+ public void testNewTempDir() {
--- End diff --
again, should be `groups="Integration"` unless your file system is very very fast?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14807606
--- Diff: utils/common/src/test/java/brooklyn/util/os/OsTest.java ---
@@ -84,5 +88,49 @@ public void testMergePaths() throws Exception {
assertEquals(Os.mergePaths("a", "b/"), "a/b/");
assertEquals(Os.mergePaths("/a", "b"), "/a/b");
}
+
+ @Test
+ public void testNewTempFile() {
--- End diff --
should be `groups="Integration"` unless your file system is very very fast?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14806334
--- Diff: core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java ---
@@ -514,10 +514,18 @@ public void defaultRemovalStrategyShutsDownNewestFirstWhenResizing() throws Exce
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);
- assertEquals(ImmutableList.copyOf(cluster.getMembers()), creationOrder.subList(0, 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());
+ }
--- End diff --
looks to me like the only way `creationOrder.subList(0,1)` would fail is if their creation times are identical (ie to nearest millisecond). i don't think there are issues with "older" one being deleted in that situation. but maybe so that we can have *both* checks (the new time one, and the original `subList` one) we should stick a `Time.sleep(Duration.millis(1))` before the `resize(2)` ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#issuecomment-48734324
LGTM - let's merge once you addressed or commented on one final comment
agree with your responses (not shown here, but i've seen them on email)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14763837
--- Diff: core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java ---
@@ -514,10 +514,18 @@ public void defaultRemovalStrategyShutsDownNewestFirstWhenResizing() throws Exce
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);
- assertEquals(ImmutableList.copyOf(cluster.getMembers()), creationOrder.subList(0, 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());
+ }
--- End diff --
Perhaps we should change the implementation to respect the insertion order instead of changing the test?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14808596
--- Diff: utils/common/src/main/java/brooklyn/util/os/Os.java ---
@@ -544,13 +545,23 @@ public static File newTempFile(Class<?> clazz, String ext) {
/** creates a temp dir which will be deleted on exit */
public static File newTempDir(String prefix) {
- String baseName = (prefix==null ? "" : prefix + "-") + Identifiers.makeRandomId(4);
- File tempDir = new File(tmp(), baseName);
- if (tempDir.mkdir()) {
- Os.deleteOnExitRecursively(tempDir);
- return tempDir;
+ String sanitizedPrefix = (prefix==null ? "" : prefix + "-");
+ String tmpParent = tmp();
+ for (int i = 0; i < TEMP_DIR_ATTEMPTS; i++) {
+ String baseName = sanitizedPrefix + Identifiers.makeRandomId(4);
+ File tempDir = new File(tmpParent, baseName);
+ if (!tempDir.exists()) {
+ if (tempDir.mkdir()) {
+ Os.deleteOnExitRecursively(tempDir);
+ return tempDir;
+ } else {
+ log.warn("Attempt to create temp dir failed " + tempDir);
+ }
+ } else {
+ log.debug("Attemp to create temp dir failed, already exists " + tempDir);
--- End diff --
> do we really want to retry 1000 times?
The implementations for createTempFile and createTempDirectory(Java 7+) loop indefinitely until a unique name is found. The guava implementation (com.google.common.io.Files.createTempDir()) retries 10 000 times.
Considering this it might be a good idea even to increase it :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix failing tests
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/61#discussion_r14806538
--- Diff: utils/common/src/main/java/brooklyn/util/os/Os.java ---
@@ -544,13 +545,23 @@ public static File newTempFile(Class<?> clazz, String ext) {
/** creates a temp dir which will be deleted on exit */
public static File newTempDir(String prefix) {
- String baseName = (prefix==null ? "" : prefix + "-") + Identifiers.makeRandomId(4);
- File tempDir = new File(tmp(), baseName);
- if (tempDir.mkdir()) {
- Os.deleteOnExitRecursively(tempDir);
- return tempDir;
+ String sanitizedPrefix = (prefix==null ? "" : prefix + "-");
+ String tmpParent = tmp();
+ for (int i = 0; i < TEMP_DIR_ATTEMPTS; i++) {
+ String baseName = sanitizedPrefix + Identifiers.makeRandomId(4);
+ File tempDir = new File(tmpParent, baseName);
+ if (!tempDir.exists()) {
+ if (tempDir.mkdir()) {
+ Os.deleteOnExitRecursively(tempDir);
+ return tempDir;
+ } else {
+ log.warn("Attempt to create temp dir failed " + tempDir);
+ }
+ } else {
+ log.debug("Attemp to create temp dir failed, already exists " + tempDir);
--- End diff --
missing a `t` in `Attempt`
worth a comment that this collision is unlikely due to the `makeRandomId` but with lots of temp dirs it can start to happen ?
if the creation failed (`log.warn`), e.g. because a directory was not writable or disk is full, do we really want to retry 1000 times? maybe set `ATTEMPTS` much lower?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---