[GitHub] [maven-resolver] cstamas opened a new pull request #77: SyncContext implementations

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
150 messages Options
12345 ... 8
Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] michael-o commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox

michael-o commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549166502



##########
File path: maven-resolver-synccontext-named/pom.xml
##########
@@ -59,7 +59,13 @@
     </dependency>
     <dependency>
       <groupId>org.apache.maven.resolver</groupId>
-      <artifactId>maven-resolver-impl</artifactId>
+      <artifactId>maven-resolver-spi</artifactId>
+      <scope>provided</scope>
+      <optional>true</optional>

Review comment:
       Why is it optional if it is provided?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] michael-o commented on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

michael-o commented on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751531066


   Very nice, I will happily go through.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] rmannibucau commented on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

rmannibucau commented on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751680455


   Hi, I have 2 questions on the PR:
   
   1. I think hazelcast should handle a property to find the client configuration file as redisson does (will avoid to play with classpath or hazelcast internal config). Aligning both/all configs sounds sane to me (maybe maven/conf/resolver.conf or so, not sure about the detail but aligning it sounds good for end users, maybe using a properties format for all and single configuration file)
   2. What about the maven classpath when using hz or redis client? Shouldn't both be shaded or (better?) isolated (using maven/resolver-lib for example and creating an URLClassLoader to execute them - or just another ClassRealm, no strong preference).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] michael-o commented on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

michael-o commented on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751682400


   @rmannibucau
   1) This makes sense, but I don't want to create a mapping from properties to the object ob Redisson. The YAML configuration is so vast that I want to leave it to the user. As you can see in my code I tried to keep it as simple as possible.
   2) That's a good question. I thougt about providing a shaded ├╝ber JAR for this, but don't know whether everything can be shaded or not.
   
   If you have ideas for 1), please provide.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751683148


   Howdy @rmannibucau
   For now I consider both HZ and Redisson as "lab" implementation still, due these:
   * for HZ to work we either must use deprecated HZ 3.x call (HazelcastInstance#getSemaphore), or have a remote CP cluster set up (here, setup is somewhat similar to Redisson: you must have some remote "thing", server, cluster set up beforehand)
   * for Redisson, no clue how to test (any ideas?)
   
   So for start, I'd focus on local (JVM local) locks and the two "extra" (again, for testing purposes: global and nolock) implementations. As they are pretty much in place. Given this PR does not include any of HZ or Redisson code into resolver (they are separate modules, it leaves it to integrator), I think we still have time to solve both questions, that IMO are valid and actually sounds useful (generic props for configuration).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas edited a comment on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas edited a comment on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751683148


   Howdy @rmannibucau
   For now I consider both HZ and Redisson as "lab" implementation still, due these:
   * for HZ to work we either must use deprecated HZ 3.x call (HazelcastInstance#getSemaphore), or have a remote CP cluster set up (here, setup is somewhat similar to Redisson: you must have some remote "thing", server, cluster set up beforehand)
   * for Redisson, no clue how to set up tests (any ideas?)
   
   So for start, I'd focus on local (JVM local) locks and the two "extra" (again, for testing purposes: global and nolock) implementations. As they are pretty much in place. Given this PR does not include any of HZ or Redisson code into resolver (they are separate modules, it leaves it to integrator), I think we still have time to solve both questions, that IMO are valid and actually sounds useful (generic props for configuration).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas edited a comment on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas edited a comment on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751683148


   Howdy @rmannibucau
   For now I consider both HZ and Redisson as "lab" implementation still, due these:
   * for HZ to work we either must use deprecated HZ 3.x call (HazelcastInstance#getSemaphore), or have a remote CP cluster set up (here, setup is somewhat similar to Redisson: you must have some remote "thing", server, cluster set up beforehand)
   * for Redisson, no clue how to set up tests (any ideas? can we use docker on Jenkins?)
   
   So for start, I'd focus on local (JVM local) locks and the two "extra" (again, for testing purposes: global and nolock) implementations. As they are pretty much in place. Given this PR does not include any of HZ or Redisson code into resolver (they are separate modules, it leaves it to integrator), I think we still have time to solve both questions, that IMO are valid and actually sounds useful (generic props for configuration).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] michael-o commented on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

michael-o commented on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751685099


   As for testing Redis, I would not rely on tools like Docker because it is too OS specific. One could probe whether the redis daemon is in `PATH`. If yes, start it. Run tests, shut it down. This won't work on Windows of course, because Redis requires a POSIX-like OS.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] rmannibucau commented on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

rmannibucau commented on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751685353


   @michael-o everything can be shaded but a few can be "hard". For properties what about supporting in the resolver.properties something like "configuration.external=path-to-conf-in-maven-base" (ex: configuration.external=conf/redisson.yaml) + if not set a minimal set of properties (host, port, ...). Kind of default simple mode and advanced mode through an external file. On the shading point, I would avoid it to keep the pluggability capabilities of external libs but I would isolate it from maven classpath (requires minimum reflection if well split, just a factory which returns a maven spi).
   
   @cstamas guess for hz we can focus on v4, nobody uses the v3 so let's not make it more complicated than needed to start as you mentionned. For redisson we can have testcontainers tests (with a flag to enable them in run-its probably since it requires docker locally and we don't want to force all dev to have it even if it is common these days). Will be easier than using a ProcessBuilder for redis and stay portable (the test can assume the OS env which is good for such simple cases). I also agree we have to focus on nolock/jvmlock/locallock (file) cases but if we consider hz/redisson as experimental then we should not put them in this repo (or enable the module for releases) IMHO.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] michael-o commented on pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

michael-o commented on pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#issuecomment-751690622


   @rmannibucau I would not exclude them from the delivery, but mark as experimental. I want people to test them. If we don't make it publically available it won't be usable for most.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] michael-o commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

michael-o commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549341209



##########
File path: maven-resolver-impl/pom.xml
##########
@@ -49,6 +49,10 @@
       <groupId>org.apache.maven.resolver</groupId>
       <artifactId>maven-resolver-spi</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.maven.resolver</groupId>
+      <artifactId>maven-resolver-named</artifactId>

Review comment:
       I think this is a bad name for an artifact id. It says that the Resolver is named. `maven-resolver-synccontext-named`?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java
##########
@@ -0,0 +1,91 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.LocalReadWriteLockProvider;
+import org.eclipse.aether.spi.synccontext.SyncContextFactory;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Named {@link SyncContextFactory} implementation that selects underlying {@link NamedLockFactory} implementation
+ * at creation.
+ */
+@Named
+@Singleton
+public final class NamedSyncContextFactory
+        implements SyncContextFactory
+{
+    private static final long TIME = Long.getLong(

Review comment:
       Isn't this rather a timeout?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();
+            LOGGER.trace( "Using key discriminator '{}' during this session", discriminator );
+
+            LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys );
+            int acquiredLockCount = 0;
+            for ( String key : keys )
+            {
+                NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX + discriminator + ":" + key );
+                try
+                {
+                    boolean locked;
+                    if ( shared )
+                    {
+                        locked = namedLock.lockShared( time, timeUnit );
+                    }
+                    else
+                    {
+                        locked = namedLock.lockExclusively( time, timeUnit );
+                    }
+
+                    if ( !locked )
+                    {
+                        namedLock.close();
+                        throw new IllegalStateException( "Could not lock "
+                                + namedLock.name() + " (shared=" + shared + ")" );
+                    }
+
+                    locks.push( namedLock );
+                    acquiredLockCount++;
+                }
+                catch ( InterruptedException e )
+                {
+                    Thread.currentThread().interrupt();
+                    throw new RuntimeException( e );
+                }
+            }
+            LOGGER.trace( "Total new locks acquired: {}", acquiredLockCount );
+        }
+
+        private String createDiscriminator()
+        {
+            String discriminator = ConfigUtils.getString( session, null, CONFIG_PROP_DISCRIMINATOR );
+
+            if ( discriminator == null || discriminator.isEmpty() )
+            {
+                File basedir = session.getLocalRepository().getBasedir();
+                discriminator = hostname + ":" + basedir;
+                try
+                {
+                    Map<String, Object> checksums = ChecksumUtils.calc(
+                            discriminator.getBytes( StandardCharsets.UTF_8 ),
+                            Collections.singletonList( "SHA-1" ) );
+                    Object checksum = checksums.get( "SHA-1" );
+
+                    if ( checksum instanceof Exception )
+                    {
+                        throw ( Exception ) checksum;
+                    }
+
+                    return String.valueOf( checksum );
+                }
+                catch ( Exception e )
+                {
+                    LOGGER.warn( "Failed to calculate discriminator digest, using '{}'",
+                            DEFAULT_DISCRIMINATOR_DIGEST, e );
+                    return DEFAULT_DISCRIMINATOR_DIGEST;
+                }
+            }
+
+            return discriminator;
+        }
+
+        @Override
+        public void close()
+        {
+            if ( locks.isEmpty() )
+            {
+                return;
+            }
+
+            // Release locks in reverse insertion order
+            int released = 0;
+            while ( !locks.isEmpty() )

Review comment:
       I understand that the array deque gives you the pop for free, but the map has more benefits.

##########
File path: maven-resolver-named-hazelcast/src/test/resources/hazelcast-client.xml
##########
@@ -0,0 +1,39 @@
+<?xml version="1.0"?>
+
+<!--
+/*
+ * 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.
+ */
+-->
+
+<hazelcast-client xmlns="http://www.hazelcast.com/schema/client-config"
+                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                  xsi:schemaLocation="http://www.hazelcast.com/schema/client-config
+                  http://www.hazelcast.com/schema/client-config/hazelcast-client-config-3.12.xsd">
+
+    <group>
+        <name>maven-resolved-named</name>
+    </group>
+    <instance-name>client</instance-name>
+    <properties>
+        <property name="hazelcast.logging.type">slf4j</property>
+    </properties>
+    <network>
+        <cluster-members>
+            <address>127.0.0.1</address>

Review comment:
       This shoulc rather be `localhost` host otherwise other IPv4 local address won't bind or `::1`.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java
##########
@@ -0,0 +1,91 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.LocalReadWriteLockProvider;
+import org.eclipse.aether.spi.synccontext.SyncContextFactory;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Named {@link SyncContextFactory} implementation that selects underlying {@link NamedLockFactory} implementation
+ * at creation.
+ */
+@Named
+@Singleton
+public final class NamedSyncContextFactory

Review comment:
       All system properties for this class should start with `aether.syncContext.named.` to scope them for Resolver property.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();
+            LOGGER.trace( "Using key discriminator '{}' during this session", discriminator );
+
+            LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys );
+            int acquiredLockCount = 0;
+            for ( String key : keys )
+            {
+                NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX + discriminator + ":" + key );
+                try
+                {
+                    boolean locked;
+                    if ( shared )
+                    {
+                        locked = namedLock.lockShared( time, timeUnit );

Review comment:
       Now this is really a timeout, not just time. I am quite certain that this will fail with 10 s when large artifacts are transferred. But OK, users should adapt or we bump to 30 s.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";

Review comment:
       This might be a matter of taste, but I rather prefix the full name here `maven:resolver:`. See https://github.com/apache/maven/pull/419/files#diff-c9096a367ccf68804df8750db73e4a5cc8493bfb3d46242442cd7418a9e110e0R248.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();
+            LOGGER.trace( "Using key discriminator '{}' during this session", discriminator );
+
+            LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys );
+            int acquiredLockCount = 0;
+            for ( String key : keys )
+            {
+                NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX + discriminator + ":" + key );
+                try
+                {
+                    boolean locked;
+                    if ( shared )
+                    {
+                        locked = namedLock.lockShared( time, timeUnit );
+                    }
+                    else
+                    {
+                        locked = namedLock.lockExclusively( time, timeUnit );
+                    }
+
+                    if ( !locked )
+                    {
+                        namedLock.close();
+                        throw new IllegalStateException( "Could not lock "
+                                + namedLock.name() + " (shared=" + shared + ")" );
+                    }
+
+                    locks.push( namedLock );

Review comment:
       Since you already cache locks, why not a the linked hash map to reuse the from for reacquision?

##########
File path: maven-resolver-named-hazelcast/src/site/site.xml
##########
@@ -22,7 +22,7 @@ under the License.
 <project xmlns="http://maven.apache.org/DECORATION/1.0.0"
   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
   xsi:schemaLocation="http://maven.apache.org/DECORATION/1.0.0 http://maven.apache.org/xsd/decoration-1.0.0.xsd"
-  name="GlobalSyncContext">
+  name="NamedSyncContext">

Review comment:
       The name says `NamedSyncContext` although this is Hazelcast?

##########
File path: maven-resolver-named-redisson/pom.xml
##########
@@ -28,51 +28,57 @@
     <version>1.7.0-SNAPSHOT</version>
   </parent>
 
-  <artifactId>maven-resolver-synccontext-redisson</artifactId>
+  <artifactId>maven-resolver-named-redisson</artifactId>

Review comment:
       See comment for `maven-resolver-named`.

##########
File path: maven-resolver-named/src/main/java/org/eclipse/aether/named/NamedLock.java
##########
@@ -0,0 +1,71 @@
+package org.eclipse.aether.named;
+
+/*
+ * 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;
+
+/**
+ * A named lock, functionally similar to existing JVM and other implementations. Does support boxing, but no
+ * lock upgrade is supported. Usual pattern to use this lock:
+ * <pre>
+ *   try (NamedLock lock = factory.getLock("resourceName")) {
+ *     if (lock.lockExclusively(10L, Timeunit.SECONDS)) {
+ *       try {
+ *         ... exclusive access to "resourceName" resource gained here
+ *       }
+ *       finally {
+ *         lock.unlock();
+ *       }
+ *     }
+ *     else {
+ *       ... failed to gain access within specified time, handle it
+ *     }
+ *   }
+ * </pre>
+ */
+public interface NamedLock

Review comment:
       The documentation should clearly state that every implementation must support reentrancy. This is required by the `SyncContextFactory`.

##########
File path: maven-resolver-named-redisson/src/site/markdown/index.md.vm
##########
@@ -0,0 +1,69 @@
+${esc.hash} Named Sync Context for Maven Resolver
+
+<!--
+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.
+-->
+
+The Named Sync Context Factory is a distributed lock factory for Maven Resolver
+on top of JVM and Hazelcast to provide a fast, concurrent-safe access from one or multiple Maven instances to the
+same local Maven repository.
+
+For further details about the factory read the [Javadoc](./apidocs/org/eclipse/aether/synccontext/NamedSyncContextFactory.html).
+
+${esc.hash}${esc.hash} Open Issues/Notes
+
+- It only works when dependency injection is used and not the bundled `AetherModule` or
+  `ServiceLocator` (Maven uses dependency injection).
+- It includes a lot of trace logging which partially will go way as soon as it has been stabilized.
+- Usage from plugins has not been tested yet.
+- The `furnace-maven-plugin` does not work this implementation because it uses `ServiceLocator` instead
+  of dependency injection.
+
+${esc.hash}${esc.hash} Installation/Testing
+
+- Create the directory `${maven.home}/lib/ext/hazelcast/`.

Review comment:
       The file path says redisson while this covers Hazelcast.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();
+            LOGGER.trace( "Using key discriminator '{}' during this session", discriminator );
+
+            LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys );
+            int acquiredLockCount = 0;
+            for ( String key : keys )
+            {
+                NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX + discriminator + ":" + key );
+                try
+                {
+                    boolean locked;
+                    if ( shared )
+                    {
+                        locked = namedLock.lockShared( time, timeUnit );
+                    }
+                    else
+                    {
+                        locked = namedLock.lockExclusively( time, timeUnit );
+                    }
+
+                    if ( !locked )
+                    {
+                        namedLock.close();
+                        throw new IllegalStateException( "Could not lock "
+                                + namedLock.name() + " (shared=" + shared + ")" );
+                    }
+
+                    locks.push( namedLock );
+                    acquiredLockCount++;
+                }
+                catch ( InterruptedException e )
+                {
+                    Thread.currentThread().interrupt();
+                    throw new RuntimeException( e );
+                }
+            }
+            LOGGER.trace( "Total new locks acquired: {}", acquiredLockCount );

Review comment:
       This isn't true anymore. The reacquisition of locks is gone while from a debugging point of view I'd like to retain it.

##########
File path: maven-resolver-named-redisson/src/site/site.xml
##########
@@ -22,7 +22,7 @@ under the License.
 <project xmlns="http://maven.apache.org/DECORATION/1.0.0"
   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
   xsi:schemaLocation="http://maven.apache.org/DECORATION/1.0.0 http://maven.apache.org/xsd/decoration-1.0.0.xsd"
-  name="RedissonSyncContext">
+  name="NamedSyncContext">

Review comment:
       Same as with Hazelcast.

##########
File path: maven-resolver-named/src/site/markdown/index.md.vm
##########
@@ -0,0 +1,69 @@
+${esc.hash} Named Sync Context for Maven Resolver
+
+<!--
+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.
+-->
+
+The Named Sync Context Factory is a distributed lock factory for Maven Resolver
+on top of JVM and Hazelcast to provide a fast, concurrent-safe access from one or multiple Maven instances to the
+same local Maven repository.
+
+For further details about the factory read the [Javadoc](./apidocs/org/eclipse/aether/synccontext/NamedSyncContextFactory.html).
+
+${esc.hash}${esc.hash} Open Issues/Notes
+
+- It only works when dependency injection is used and not the bundled `AetherModule` or
+  `ServiceLocator` (Maven uses dependency injection).
+- It includes a lot of trace logging which partially will go way as soon as it has been stabilized.
+- Usage from plugins has not been tested yet.
+- The `furnace-maven-plugin` does not work this implementation because it uses `ServiceLocator` instead
+  of dependency injection.
+
+${esc.hash}${esc.hash} Installation/Testing
+
+- Create the directory `${maven.home}/lib/ext/hazelcast/`.

Review comment:
       Same as the other file...

##########
File path: maven-resolver-named-hazelcast/src/test/resources/hazelcast.xml
##########
@@ -0,0 +1,42 @@
+<?xml version="1.0"?>
+
+<!--
+/*
+ * 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.
+ */
+-->
+
+<hazelcast xmlns="http://www.hazelcast.com/schema/config"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xsi:schemaLocation="http://www.hazelcast.com/schema/config
+           http://www.hazelcast.com/schema/config/hazelcast-config-3.12.xsd">
+
+    <group>
+        <name>maven-resolved-named</name>
+    </group>
+    <instance-name>server</instance-name>
+    <properties>
+        <property name="hazelcast.logging.type">slf4j</property>
+    </properties>
+    <network>
+        <join>
+            <multicast enabled="false"/>
+            <tcp-ip>
+                <interface>127.0.0.1</interface>

Review comment:
       Same here

##########
File path: maven-resolver-named/src/main/java/org/eclipse/aether/named/providers/GlobalReadWriteLockProvider.java
##########
@@ -0,0 +1,129 @@
+package org.eclipse.aether.named.providers;
+
+/*
+ * 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 org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.ArrayDeque;
+import java.util.NoSuchElementException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+/**
+ * Provider of {@link GlobalNamedLockFactory}. This uses one "global" {@link ReentrantReadWriteLock}, is really not a
+ * "named" lock.
+ */
+@Singleton
+@Named( GlobalReadWriteLockProvider.NAME )
+public final class GlobalReadWriteLockProvider
+        implements Provider<NamedLockFactory>
+{
+    public static final String NAME = "global";
+
+    @Override
+    public NamedLockFactory get()
+    {
+        return new GlobalNamedLockFactory();
+    }
+
+    /**
+     * A global named lock factory that actually uses one singleton named lock instance for all names.
+     */
+    public static final class GlobalNamedLockFactory implements NamedLockFactory
+    {
+        private final GlobalNamedLock globalNamedLock = new GlobalNamedLock();
+
+        @Override
+        public NamedLock getLock( String name )
+        {
+            return globalNamedLock;
+        }
+
+        @Override
+        public void shutdown()
+        {
+            // nop
+        }
+    }
+
+    /**
+     * A global named lock that uses one single reentrant read-write lock.
+     */
+    public static final class GlobalNamedLock implements NamedLock
+    {
+        private final ReentrantReadWriteLock global = new ReentrantReadWriteLock();
+
+        private final ArrayDeque<Lock> steps = new ArrayDeque<>();

Review comment:
       Does the deque serve as hold count for reeentrant calls like in my Redisson?
   ```
                   while ( actualLock.getHoldCount() > 0 )
                   {
                       // Avoid #getHoldCount() roundtrips when we are not logging
                       if ( LOGGER.isTraceEnabled() )
                       {
                           LOGGER.trace( "Releasing {} lock for '{}' (currently held: {})",
                                         shared ? "read" : "write", key, actualLock.getHoldCount() );
                       }
                       actualLock.unlock();
                   }
   ```

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();

Review comment:
       I think having this here is wrong. This is an inplementation detail of a concrete named provider and not a general one. Mostly distirbuted locking systems need it. In-VM don't require it. I would prefer that this classes uses external lock names while a named provider can tranform this to a internal one.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549457200



##########
File path: maven-resolver-impl/pom.xml
##########
@@ -49,6 +49,10 @@
       <groupId>org.apache.maven.resolver</groupId>
       <artifactId>maven-resolver-spi</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.maven.resolver</groupId>
+      <artifactId>maven-resolver-named</artifactId>

Review comment:
       "synccontext" exists in resolver-api (SyncContext) and resolver-spi (SyncContextFactory). While I agree that name maven-resolver-named is misleading, it has none of those two contained. What about maven-resolver-named-locks? It was my initial idea, but abstained as then maven-resolver-named-locks-redisson would be unusually long artifact ID...




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549460660



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java
##########
@@ -0,0 +1,91 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.LocalReadWriteLockProvider;
+import org.eclipse.aether.spi.synccontext.SyncContextFactory;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Named {@link SyncContextFactory} implementation that selects underlying {@link NamedLockFactory} implementation
+ * at creation.
+ */
+@Named
+@Singleton
+public final class NamedSyncContextFactory

Review comment:
       Fixed




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549460708



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";

Review comment:
       Fixed

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java
##########
@@ -0,0 +1,91 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.LocalReadWriteLockProvider;
+import org.eclipse.aether.spi.synccontext.SyncContextFactory;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Named {@link SyncContextFactory} implementation that selects underlying {@link NamedLockFactory} implementation
+ * at creation.
+ */
+@Named
+@Singleton
+public final class NamedSyncContextFactory
+        implements SyncContextFactory
+{
+    private static final long TIME = Long.getLong(

Review comment:
       Fixed




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549460800



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();
+            LOGGER.trace( "Using key discriminator '{}' during this session", discriminator );
+
+            LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys );
+            int acquiredLockCount = 0;
+            for ( String key : keys )
+            {
+                NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX + discriminator + ":" + key );
+                try
+                {
+                    boolean locked;
+                    if ( shared )
+                    {
+                        locked = namedLock.lockShared( time, timeUnit );

Review comment:
       Fixed both: renamed and upped to 30s




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549461252



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();
+            LOGGER.trace( "Using key discriminator '{}' during this session", discriminator );
+
+            LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys );
+            int acquiredLockCount = 0;
+            for ( String key : keys )
+            {
+                NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX + discriminator + ":" + key );
+                try
+                {
+                    boolean locked;
+                    if ( shared )
+                    {
+                        locked = namedLock.lockShared( time, timeUnit );
+                    }
+                    else
+                    {
+                        locked = namedLock.lockExclusively( time, timeUnit );
+                    }
+
+                    if ( !locked )
+                    {
+                        namedLock.close();
+                        throw new IllegalStateException( "Could not lock "
+                                + namedLock.name() + " (shared=" + shared + ")" );
+                    }
+
+                    locks.push( namedLock );
+                    acquiredLockCount++;
+                }
+                catch ( InterruptedException e )
+                {
+                    Thread.currentThread().interrupt();
+                    throw new RuntimeException( e );
+                }
+            }
+            LOGGER.trace( "Total new locks acquired: {}", acquiredLockCount );

Review comment:
       Removed "new" (the untrue part).




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549461325



##########
File path: maven-resolver-named-hazelcast/src/test/resources/hazelcast-client.xml
##########
@@ -0,0 +1,39 @@
+<?xml version="1.0"?>
+
+<!--
+/*
+ * 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.
+ */
+-->
+
+<hazelcast-client xmlns="http://www.hazelcast.com/schema/client-config"
+                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                  xsi:schemaLocation="http://www.hazelcast.com/schema/client-config
+                  http://www.hazelcast.com/schema/client-config/hazelcast-client-config-3.12.xsd">
+
+    <group>
+        <name>maven-resolved-named</name>
+    </group>
+    <instance-name>client</instance-name>
+    <properties>
+        <property name="hazelcast.logging.type">slf4j</property>
+    </properties>
+    <network>
+        <cluster-members>
+            <address>127.0.0.1</address>

Review comment:
       Fixed




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549461341



##########
File path: maven-resolver-named-hazelcast/src/test/resources/hazelcast.xml
##########
@@ -0,0 +1,42 @@
+<?xml version="1.0"?>
+
+<!--
+/*
+ * 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.
+ */
+-->
+
+<hazelcast xmlns="http://www.hazelcast.com/schema/config"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xsi:schemaLocation="http://www.hazelcast.com/schema/config
+           http://www.hazelcast.com/schema/config/hazelcast-config-3.12.xsd">
+
+    <group>
+        <name>maven-resolved-named</name>
+    </group>
+    <instance-name>server</instance-name>
+    <properties>
+        <property name="hazelcast.logging.type">slf4j</property>
+    </properties>
+    <network>
+        <join>
+            <multicast enabled="false"/>
+            <tcp-ip>
+                <interface>127.0.0.1</interface>

Review comment:
       Fixed




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549461637



##########
File path: maven-resolver-named/src/main/java/org/eclipse/aether/named/NamedLock.java
##########
@@ -0,0 +1,71 @@
+package org.eclipse.aether.named;
+
+/*
+ * 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;
+
+/**
+ * A named lock, functionally similar to existing JVM and other implementations. Does support boxing, but no
+ * lock upgrade is supported. Usual pattern to use this lock:
+ * <pre>
+ *   try (NamedLock lock = factory.getLock("resourceName")) {
+ *     if (lock.lockExclusively(10L, Timeunit.SECONDS)) {
+ *       try {
+ *         ... exclusive access to "resourceName" resource gained here
+ *       }
+ *       finally {
+ *         lock.unlock();
+ *       }
+ *     }
+ *     else {
+ *       ... failed to gain access within specified time, handle it
+ *     }
+ *   }
+ * </pre>
+ */
+public interface NamedLock

Review comment:
       That was already stated by "boxing". Reworded and added MUST and clarification, as it sounds clearer.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-resolver] cstamas commented on a change in pull request #77: [MRESOLVER-145] SyncContext implementations

GitBox
In reply to this post by GitBox

cstamas commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549467158



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * 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 org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+    private static final String DEFAULT_DISCRIMINATOR_DIGEST = "da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+    private static final String DEFAULT_HOSTNAME = "localhost";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( SyncContextFactoryAdapter.class );
+
+    private final NamedLockFactory namedLockFactory;
+
+    private final long time;
+
+    private final TimeUnit timeUnit;
+
+    private final String hostname;
+
+    public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+                                      final long time,
+                                      final TimeUnit timeUnit )
+    {
+        this.namedLockFactory = namedLockFactory;
+        this.time = time;
+        this.timeUnit = timeUnit;
+        this.hostname = getHostname();
+    }
+
+    private String getHostname()
+    {
+        try
+        {
+            return InetAddress.getLocalHost().getHostName();
+        }
+        catch ( UnknownHostException e )
+        {
+            LOGGER.warn( "Failed to get hostname, using '{}'", DEFAULT_HOSTNAME, e );
+            return DEFAULT_HOSTNAME;
+        }
+    }
+
+    public SyncContext newInstance( final RepositorySystemSession session, final boolean shared )
+    {
+        return new AdaptedLockSyncContext( session, hostname, namedLockFactory, time, timeUnit, shared );
+    }
+
+    public void shutdown()
+    {
+        namedLockFactory.shutdown();
+    }
+
+    private static class AdaptedLockSyncContext
+            implements SyncContext
+    {
+        private static final String CONFIG_PROP_DISCRIMINATOR = "aether.syncContext.named.discriminator";
+
+        private static final String KEY_PREFIX = "mvn:resolver:";
+
+        private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class );
+
+        private final RepositorySystemSession session;
+
+        private final String hostname;
+
+        private final NamedLockFactory namedLockFactory;
+
+        private final long time;
+
+        private final TimeUnit timeUnit;
+
+        private final boolean shared;
+
+        private final ArrayDeque<NamedLock> locks;
+
+        private AdaptedLockSyncContext( final RepositorySystemSession session,
+                                        final String hostname,
+                                        final NamedLockFactory namedLockFactory,
+                                        final long time,
+                                        final TimeUnit timeUnit,
+                                        final boolean shared )
+        {
+            this.session = session;
+            this.hostname = hostname;
+            this.namedLockFactory = namedLockFactory;
+            this.time = time;
+            this.timeUnit = timeUnit;
+            this.shared = shared;
+            this.locks = new ArrayDeque<>();
+        }
+
+        @Override
+        public void acquire( Collection<? extends Artifact> artifacts,
+                             Collection<? extends Metadata> metadatas )
+        {
+            // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+            // We must acquire multiple locks always in the same order!
+            Collection<String> keys = new TreeSet<>();
+            if ( artifacts != null )
+            {
+                for ( Artifact artifact : artifacts )
+                {
+                    // TODO Should we include extension and classifier too?
+                    String key = "artifact:" + artifact.getGroupId() + ":"
+                            + artifact.getArtifactId() + ":" + artifact.getBaseVersion();
+                    keys.add( key );
+                }
+            }
+
+            if ( metadatas != null )
+            {
+                for ( Metadata metadata : metadatas )
+                {
+                    StringBuilder key = new StringBuilder( "metadata:" );
+                    if ( !metadata.getGroupId().isEmpty() )
+                    {
+                        key.append( metadata.getGroupId() );
+                        if ( !metadata.getArtifactId().isEmpty() )
+                        {
+                            key.append( ':' ).append( metadata.getArtifactId() );
+                            if ( !metadata.getVersion().isEmpty() )
+                            {
+                                key.append( ':' ).append( metadata.getVersion() );
+                            }
+                        }
+                    }
+                    keys.add( key.toString() );
+                }
+            }
+
+            if ( keys.isEmpty() )
+            {
+                return;
+            }
+
+            String discriminator = createDiscriminator();

Review comment:
       Unsure about this... while I tend to agree, what happens in cases. like mvnd, where muitiple executions (potentially having mutiple settings XML used) may be executing? (mvnd is new to me, unsure what and how it really solve this). In that case localRepo1:GAV and localRepo2:GAV are two separate things, no?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


12345 ... 8