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:54:30 UTC

[GitHub] [maven-resolver] cstamas commented on a diff in pull request #276: [MRESOLVER-349] Adapter should give up and retry

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