[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
123456 ... 8
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

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



##########
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:
       Why complicate? That's the point why factory caches locks (so getting already acquired ones is cheap), and the benefit is that client code (this code) is straightforward, simple, does not have hops like if-this-then-else-that... this code in adapter does not have to care at all about all those nuances 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] 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_r549481196



##########
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:
       Again, why complicate?




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



##########
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:
       renamed "named"->"named-locks"




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



##########
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:
       Fixed

##########
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:
       Renamed

##########
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:
       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_r550303703



##########
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:
       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_r550303909



##########
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:
       This class is removed




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



##########
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:
       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] 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-754170465


   Will pick up by Wednesday.


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


   @michael-o @rmannibucau My proposal is to merge this PR, we already know current master is broken (see MRESOLVER-153), and while in this PR I did "return" (well, copied to new place) the `NoLockSyncContextFactory` (the `DefaultSyncContextFactory` that was noop, cause of MRESOLVER-153) I am now tempted to drop it, and merge.
   WDYT?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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


   I need to go through another iteration because I wasn't really happy with some things last time, plus I need test myself too. I am currently on a short leash, but will try to go through this month.


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


   I need to go through another iteration because I wasn't really happy with some things last time, plus I need test myself too. I am currently on a short leash, but will try to go through this month.


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


   @michael-o @rmannibucau My proposal is to merge this PR, we already know current master is broken (see MRESOLVER-153), and while in this PR I did "return" (well, copied to new place) the `NoLockSyncContextFactory` (the `DefaultSyncContextFactory` that was noop, cause of MRESOLVER-153) I am now tempted to drop it, and merge.
   WDYT?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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_r568863257



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java
##########
@@ -22,39 +22,42 @@
 import java.util.Collection;
 
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.SyncContext;
 import org.eclipse.aether.artifact.Artifact;
-import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.eclipse.aether.metadata.Metadata;
 
 /**
  * A factory to create synchronization contexts. This default implementation does not provide any real
  * synchronization but merely completes the repository system.
  */
-@Named
-public class DefaultSyncContextFactory
-    implements SyncContextFactory, org.eclipse.aether.impl.SyncContextFactory
+@Singleton
+@Named( NoLockSyncContextFactory.NAME )
+public class NoLockSyncContextFactory

Review comment:
       Frankly, I'd drop this class, as current master (and 1.6.x as for that matter) suffers from MRESOVER-153, so this class is really an implementation "we know to cause problems".




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



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java
##########
@@ -22,39 +22,42 @@
 import java.util.Collection;
 
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.SyncContext;
 import org.eclipse.aether.artifact.Artifact;
-import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.eclipse.aether.metadata.Metadata;
 
 /**
  * A factory to create synchronization contexts. This default implementation does not provide any real
  * synchronization but merely completes the repository system.
  */
-@Named
-public class DefaultSyncContextFactory
-    implements SyncContextFactory, org.eclipse.aether.impl.SyncContextFactory
+@Singleton
+@Named( NoLockSyncContextFactory.NAME )
+public class NoLockSyncContextFactory

Review comment:
       That is absolutely correct, but shouldn't it remain for testing purposes and proof that real code works?




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



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java
##########
@@ -22,39 +22,42 @@
 import java.util.Collection;
 
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.SyncContext;
 import org.eclipse.aether.artifact.Artifact;
-import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.eclipse.aether.metadata.Metadata;
 
 /**
  * A factory to create synchronization contexts. This default implementation does not provide any real
  * synchronization but merely completes the repository system.
  */
-@Named
-public class DefaultSyncContextFactory
-    implements SyncContextFactory, org.eclipse.aether.impl.SyncContextFactory
+@Singleton
+@Named( NoLockSyncContextFactory.NAME )
+public class NoLockSyncContextFactory

Review comment:
       If you really want to drop, just do, but I think it has its usecases.




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



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java
##########
@@ -22,39 +22,42 @@
 import java.util.Collection;
 
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.SyncContext;
 import org.eclipse.aether.artifact.Artifact;
-import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.eclipse.aether.metadata.Metadata;
 
 /**
  * A factory to create synchronization contexts. This default implementation does not provide any real
  * synchronization but merely completes the repository system.
  */
-@Named
-public class DefaultSyncContextFactory
-    implements SyncContextFactory, org.eclipse.aether.impl.SyncContextFactory
+@Singleton
+@Named( NoLockSyncContextFactory.NAME )
+public class NoLockSyncContextFactory

Review comment:
       That is absolutely correct, but shouldn't it remain for testing purposes and proof that real code works?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java
##########
@@ -22,39 +22,42 @@
 import java.util.Collection;
 
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.SyncContext;
 import org.eclipse.aether.artifact.Artifact;
-import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.eclipse.aether.metadata.Metadata;
 
 /**
  * A factory to create synchronization contexts. This default implementation does not provide any real
  * synchronization but merely completes the repository system.
  */
-@Named
-public class DefaultSyncContextFactory
-    implements SyncContextFactory, org.eclipse.aether.impl.SyncContextFactory
+@Singleton
+@Named( NoLockSyncContextFactory.NAME )
+public class NoLockSyncContextFactory

Review comment:
       If you really want to drop, just do, but I think it has its usecases.




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



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java
##########
@@ -22,39 +22,42 @@
 import java.util.Collection;
 
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.SyncContext;
 import org.eclipse.aether.artifact.Artifact;
-import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.eclipse.aether.metadata.Metadata;
 
 /**
  * A factory to create synchronization contexts. This default implementation does not provide any real
  * synchronization but merely completes the repository system.
  */
-@Named
-public class DefaultSyncContextFactory
-    implements SyncContextFactory, org.eclipse.aether.impl.SyncContextFactory
+@Singleton
+@Named( NoLockSyncContextFactory.NAME )
+public class NoLockSyncContextFactory

Review comment:
       Frankly, I'd drop this class, as current master (and 1.6.x as for that matter) suffers from MRESOVER-153, so this class is really an implementation "we know to cause problems".




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



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java
##########
@@ -22,39 +22,42 @@
 import java.util.Collection;
 
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.SyncContext;
 import org.eclipse.aether.artifact.Artifact;
-import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.eclipse.aether.metadata.Metadata;
 
 /**
  * A factory to create synchronization contexts. This default implementation does not provide any real
  * synchronization but merely completes the repository system.
  */
-@Named
-public class DefaultSyncContextFactory
-    implements SyncContextFactory, org.eclipse.aether.impl.SyncContextFactory
+@Singleton
+@Named( NoLockSyncContextFactory.NAME )
+public class NoLockSyncContextFactory

Review comment:
       kk, will leave it as is




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


   @michael-o ping


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


   Thanks for the ping. Will pick up again during this week.


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


123456 ... 8