You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/11/16 08:45:35 UTC
[GitHub] [maven-resolver] gnodet commented on a change in pull request #131: [MRESOLVER-219] File advisory locking based NamedLocks
gnodet commented on a change in pull request #131:
URL: https://github.com/apache/maven-resolver/pull/131#discussion_r750037915
##########
File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/Retry.java
##########
@@ -0,0 +1,98 @@
+package org.eclipse.aether.named.support;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Retry helper: retries operation for given time. Assumes {@code null} as "no answer". It retries as long as it
+ * gets non-null result from operation, or the given time passes.
+ *
+ * @since TBD
+ */
+public final class Retry
+{
+ private static final long DEFAULT_SLEEP_MILLIS = 100L;
+
+ private static final Logger LOGGER = LoggerFactory.getLogger( Retry.class );
+
+ private Retry()
+ {
+ // no instances
+ }
+
+ /**
+ * Same as {@link #retry(long, TimeUnit, long, Supplier, Object)} but uses {@link #DEFAULT_SLEEP_MILLIS} for
+ * {@code sleepMillis} parameter.
+ */
+ public static <R> R retry( final long time,
+ final TimeUnit unit,
+ final Supplier<R> operation,
Review comment:
Same as above, maybe it we could use a `java.time.Duration` ?
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelector.java
##########
@@ -19,22 +19,33 @@
* under the License.
*/
-import org.eclipse.aether.RepositorySystemSession;
-import org.eclipse.aether.named.NamedLock;
import org.eclipse.aether.named.NamedLockFactory;
+import java.util.concurrent.TimeUnit;
+
/**
- * A {@link NamedLockFactory} that wants to make use of {@link RepositorySystemSession}.
+ * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentially
+ * all the named locks configuration is here. Implementations may use different strategies to perform selection.
*/
-public interface SessionAwareNamedLockFactory extends NamedLockFactory
+public interface NamedLockFactorySelector
{
/**
- * Creates or reuses existing {@link NamedLock}. Returns instance MUST BE treated as "resource", best in
- * try-with-resource block.
- *
- * @param session the repository system session, must not be {@code null}
- * @param name the lock name, must not be {@code null}
- * @return named the lock instance, never {@code null}
+ * Returns the value of wait time, how much a lock blocks, must be greater than 0.
+ */
+ long waitTime();
+
+ /**
+ * Returns the time unit of {@link #waitTime()} value, never null.
+ */
+ TimeUnit waitTimeUnit();
+
Review comment:
Would it make sense to use a `java.time.Duration` instead of a `long`/`TimeUnit` pair in two separate fields...
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java
##########
@@ -19,36 +19,32 @@
* under the License.
*/
-import java.util.HashMap;
-import java.util.Map;
-import java.util.concurrent.TimeUnit;
-
-import javax.inject.Inject;
-import javax.inject.Named;
-import javax.inject.Singleton;
-
-import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper;
-import org.eclipse.aether.internal.impl.synccontext.named.GAVNameMapper;
-import org.eclipse.aether.internal.impl.synccontext.named.NameMapper;
-import org.eclipse.aether.internal.impl.synccontext.named.StaticNameMapper;
import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.FileLockNamedLockFactory;
import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory;
import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory;
import org.eclipse.aether.named.providers.NoopNamedLockFactory;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
/**
- * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentially
- * all the named locks configuration is here.
+ * Simple selector implementation that uses Java system properties and sane default values.
*/
@Singleton
@Named
-public final class NamedLockFactorySelector
+public final class SimpleNamedLockFactorySelector
+ implements NamedLockFactorySelector
{
- public static final long TIME = Long.getLong(
+ private static final long TIME = Long.getLong(
"aether.syncContext.named.time", 30L
);
- public static final TimeUnit TIME_UNIT = TimeUnit.valueOf( System.getProperty(
+ private static final TimeUnit TIME_UNIT = TimeUnit.valueOf( System.getProperty(
"aether.syncContext.named.time.unit", TimeUnit.SECONDS.name()
) );
Review comment:
Same here...
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java
##########
@@ -19,36 +19,32 @@
* under the License.
*/
-import java.util.HashMap;
-import java.util.Map;
-import java.util.concurrent.TimeUnit;
-
-import javax.inject.Inject;
-import javax.inject.Named;
-import javax.inject.Singleton;
-
-import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper;
-import org.eclipse.aether.internal.impl.synccontext.named.GAVNameMapper;
-import org.eclipse.aether.internal.impl.synccontext.named.NameMapper;
-import org.eclipse.aether.internal.impl.synccontext.named.StaticNameMapper;
import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.FileLockNamedLockFactory;
import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory;
import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory;
import org.eclipse.aether.named.providers.NoopNamedLockFactory;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
/**
- * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentially
- * all the named locks configuration is here.
+ * Simple selector implementation that uses Java system properties and sane default values.
*/
@Singleton
@Named
-public final class NamedLockFactorySelector
+public final class SimpleNamedLockFactorySelector
+ implements NamedLockFactorySelector
{
- public static final long TIME = Long.getLong(
+ private static final long TIME = Long.getLong(
"aether.syncContext.named.time", 30L
);
Review comment:
Could we avoid initializing static values using system properties ? If you think about `mvnd`, system properties are modified before running maven, but a static field can't be re-initialized...
--
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