You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/04/05 09:32:06 UTC
[GitHub] [maven-resolver] cstamas opened a new pull request, #276: [MRESOLVER-349] Adapter should give up and retry
cstamas opened a new pull request, #276:
URL: https://github.com/apache/maven-resolver/pull/276
The adapter should give up and retry if it cannot lock any specified lock within given timeframe.
---
https://issues.apache.org/jira/browse/MRESOLVER-349
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] michael-o commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #276:
URL: https://github.com/apache/maven-resolver/pull/276#discussion_r1158295886
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
Review Comment:
Same noted below! :-D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #276:
URL: https://github.com/apache/maven-resolver/pull/276#discussion_r1158294798
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
Review Comment:
attempt != retry. "2 retries" means "3 attemptes". "0 retries" means "1 attempt" (like before). But agreed, will clear up variable names here.
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
Review Comment:
attempt != retry. "2 retries" means "3 attempts". "0 retries" means "1 attempt" (like before). But agreed, will clear up variable names here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] michael-o commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #276:
URL: https://github.com/apache/maven-resolver/pull/276#discussion_r1158323839
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +151,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetry(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRY, RETRY_KEY);
+ }
+
+ private long getRetryWait(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_WAIT, RETRY_WAIT_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
+ final int attempts = retry + 1;
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int attempt = 1; attempt <= attempts; attempt++) {
Review Comment:
This looks much better now!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas merged pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #276:
URL: https://github.com/apache/maven-resolver/pull/276
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] michael-o commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #276:
URL: https://github.com/apache/maven-resolver/pull/276#discussion_r1158276327
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
+ LOGGER.trace(
+ "Attempt {}: Need {} {} lock(s) for {}", retry, keys.size(), shared ? "read" : "write", keys);
+ int acquiredLockCount = 0;
+ for (String key : keys) {
+ NamedLock namedLock = namedLockFactory.getLock(key);
+ try {
+ if (retry > 0) {
+ Thread.sleep(retrySleep);
+ }
+ LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
+
+ boolean locked;
+ if (shared) {
+ locked = namedLock.lockShared(time, timeUnit);
+ } else {
+ locked = namedLock.lockExclusively(time, timeUnit);
+ }
+
+ if (!locked) {
+ String timeStr = time + " " + timeUnit;
+ LOGGER.trace(
+ "Failed to acquire {} lock for '{}' in {}",
+ shared ? "read" : "write",
+ key,
+ timeStr);
Review Comment:
This one I like!
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
+ LOGGER.trace(
+ "Attempt {}: Need {} {} lock(s) for {}", retry, keys.size(), shared ? "read" : "write", keys);
+ int acquiredLockCount = 0;
+ for (String key : keys) {
+ NamedLock namedLock = namedLockFactory.getLock(key);
+ try {
+ if (retry > 0) {
+ Thread.sleep(retrySleep);
+ }
+ LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
+
+ boolean locked;
+ if (shared) {
+ locked = namedLock.lockShared(time, timeUnit);
+ } else {
+ locked = namedLock.lockExclusively(time, timeUnit);
+ }
+
+ if (!locked) {
+ String timeStr = time + " " + timeUnit;
+ LOGGER.trace(
+ "Failed to acquire {} lock for '{}' in {}",
+ shared ? "read" : "write",
+ key,
+ timeStr);
+
+ namedLock.close();
+ closeAll();
+ illegalStateExceptions.add(new IllegalStateException(
+ "Attempt: " + retry + ": Could not acquire " + (shared ? "read" : "write")
Review Comment:
`Attempt " + retry + ":` superfluous colon
##########
src/site/markdown/configuration.md:
##########
@@ -92,7 +92,9 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.syncContext.named.factory` | String | Name of the named lock factory implementing the `org.eclipse.aether.named.NamedLockFactory` interface. | `"rwlock-local"` | no
`aether.syncContext.named.hashing.depth` | int | The directory depth to "spread" hashes in git-like fashion, integer between 0 and 4 (inclusive). | 2 | no
`aether.syncContext.named.nameMapper` | String | Name of name mapper implementing the `org.eclipse.aether.internal.impl.synccontext.named.NameMapper` interface. | `"gav"` | no
-`aether.syncContext.named.time` | long | Amount of time a synchronization context shall wait to obtain a lock. | 30 | no
+`aether.syncContext.named.retries` | int | Count of retries SyncContext adapter should attempt, when obtaining locks. | `2` | no
+`aether.syncContext.named.retrySleep` | long | Count of milliseconds of thread to sleep between attempts, when obtaining locks. | `200` | no
Review Comment:
* Maybe subordinate? `aether.syncContext.named.retry.wait`? We use "wait" already, consistency.
* Amount of milliseconds a thread to wait between retries, when obtaining locks.
You need to clarify wether you want to use the term "attempts" or "retries" because they are not the same.
My understanding:
* Attempt: 2. Means first and second run
* Retries: 1 + 2 because the first is a try, subsequent are tries.
I think you mean attempts here.
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
Review Comment:
This doesn't look right. It should be: `for (int retry = 1; retry <= retries; retry++)` because what is attempt 0?
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
+ LOGGER.trace(
+ "Attempt {}: Need {} {} lock(s) for {}", retry, keys.size(), shared ? "read" : "write", keys);
+ int acquiredLockCount = 0;
+ for (String key : keys) {
+ NamedLock namedLock = namedLockFactory.getLock(key);
+ try {
+ if (retry > 0) {
Review Comment:
Then `> 1`
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java:
##########
@@ -130,47 +145,78 @@ private TimeUnit getTimeUnit(final RepositorySystemSession session) {
return TimeUnit.valueOf(ConfigUtils.getString(session, DEFAULT_TIME_UNIT.name(), TIME_UNIT_KEY));
}
+ private int getRetries(final RepositorySystemSession session) {
+ return ConfigUtils.getInteger(session, DEFAULT_RETRIES, RETRIES_KEY);
+ }
+
+ private long getRetrySleep(final RepositorySystemSession session) {
+ return ConfigUtils.getLong(session, DEFAULT_RETRY_SLEEP, RETRY_SLEEP_KEY);
+ }
+
@Override
public void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
Collection<String> keys = lockNaming.nameLocks(session, artifacts, metadatas);
if (keys.isEmpty()) {
return;
}
- LOGGER.trace("Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys);
- int acquiredLockCount = 0;
- for (String key : keys) {
- NamedLock namedLock = namedLockFactory.getLock(key);
- try {
- LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace("Failed to acquire {} lock for '{}'", shared ? "read" : "write", key);
-
- namedLock.close();
- throw new IllegalStateException("Could not acquire " + (shared ? "read" : "write")
- + " lock for '" + namedLock.name() + "'");
+ final ArrayList<IllegalStateException> illegalStateExceptions = new ArrayList<>();
+ for (int retry = 0; retry <= retries; retry++) {
+ LOGGER.trace(
+ "Attempt {}: Need {} {} lock(s) for {}", retry, keys.size(), shared ? "read" : "write", keys);
+ int acquiredLockCount = 0;
+ for (String key : keys) {
+ NamedLock namedLock = namedLockFactory.getLock(key);
+ try {
+ if (retry > 0) {
+ Thread.sleep(retrySleep);
+ }
+ LOGGER.trace("Acquiring {} lock for '{}'", shared ? "read" : "write", key);
+
+ boolean locked;
+ if (shared) {
+ locked = namedLock.lockShared(time, timeUnit);
+ } else {
+ locked = namedLock.lockExclusively(time, timeUnit);
+ }
+
+ if (!locked) {
+ String timeStr = time + " " + timeUnit;
+ LOGGER.trace(
+ "Failed to acquire {} lock for '{}' in {}",
+ shared ? "read" : "write",
+ key,
+ timeStr);
+
+ namedLock.close();
+ closeAll();
+ illegalStateExceptions.add(new IllegalStateException(
+ "Attempt: " + retry + ": Could not acquire " + (shared ? "read" : "write")
+ + " lock for '" + namedLock.name() + "' in " + timeStr));
+ break;
+ } else {
+ locks.push(namedLock);
+ acquiredLockCount++;
+ }
+
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
}
-
- locks.push(namedLock);
- acquiredLockCount++;
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- throw new RuntimeException(e);
}
+ LOGGER.trace("Attempt {}: Total locks acquired: {}", retry, acquiredLockCount);
+ if (acquiredLockCount == keys.size()) {
+ break;
+ }
Review Comment:
Circuit breaker here, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #276:
URL: https://github.com/apache/maven-resolver/pull/276#discussion_r1158296393
##########
src/site/markdown/configuration.md:
##########
@@ -92,7 +92,9 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.syncContext.named.factory` | String | Name of the named lock factory implementing the `org.eclipse.aether.named.NamedLockFactory` interface. | `"rwlock-local"` | no
`aether.syncContext.named.hashing.depth` | int | The directory depth to "spread" hashes in git-like fashion, integer between 0 and 4 (inclusive). | 2 | no
`aether.syncContext.named.nameMapper` | String | Name of name mapper implementing the `org.eclipse.aether.internal.impl.synccontext.named.NameMapper` interface. | `"gav"` | no
-`aether.syncContext.named.time` | long | Amount of time a synchronization context shall wait to obtain a lock. | 30 | no
+`aether.syncContext.named.retries` | int | Count of retries SyncContext adapter should attempt, when obtaining locks. | `2` | no
+`aether.syncContext.named.retrySleep` | long | Count of milliseconds of thread to sleep between attempts, when obtaining locks. | `200` | no
Review Comment:
No, I mean retries here. 0 attempts makes no sense "do not even try?"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] michael-o commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #276:
URL: https://github.com/apache/maven-resolver/pull/276#discussion_r1158325295
##########
src/site/markdown/configuration.md:
##########
@@ -92,7 +92,9 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.syncContext.named.factory` | String | Name of the named lock factory implementing the `org.eclipse.aether.named.NamedLockFactory` interface. | `"rwlock-local"` | no
`aether.syncContext.named.hashing.depth` | int | The directory depth to "spread" hashes in git-like fashion, integer between 0 and 4 (inclusive). | 2 | no
`aether.syncContext.named.nameMapper` | String | Name of name mapper implementing the `org.eclipse.aether.internal.impl.synccontext.named.NameMapper` interface. | `"gav"` | no
-`aether.syncContext.named.time` | long | Amount of time a synchronization context shall wait to obtain a lock. | 30 | no
+`aether.syncContext.named.retry` | int | Count of retries SyncContext adapter should attempt, when obtaining locks. | `2` | no
Review Comment:
Note that this can triple the wait time for the lock. It think the default value should be 1, no? WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #276:
URL: https://github.com/apache/maven-resolver/pull/276#discussion_r1158326240
##########
src/site/markdown/configuration.md:
##########
@@ -92,7 +92,9 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.syncContext.named.factory` | String | Name of the named lock factory implementing the `org.eclipse.aether.named.NamedLockFactory` interface. | `"rwlock-local"` | no
`aether.syncContext.named.hashing.depth` | int | The directory depth to "spread" hashes in git-like fashion, integer between 0 and 4 (inclusive). | 2 | no
`aether.syncContext.named.nameMapper` | String | Name of name mapper implementing the `org.eclipse.aether.internal.impl.synccontext.named.NameMapper` interface. | `"gav"` | no
-`aether.syncContext.named.time` | long | Amount of time a synchronization context shall wait to obtain a lock. | 30 | no
+`aether.syncContext.named.retry` | int | Count of retries SyncContext adapter should attempt, when obtaining locks. | `2` | no
Review Comment:
yup, agreed. _Ideally_ it should be 0, but this gives opportunity to tune it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org