[GitHub] [maven-resolver] cstamas commented on a change in pull request #101: [MRESOLVER-153] Make TrackingFileManager use NamedLocks

[GitBox <gi...@apache.org>]

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



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java
##########
@@ -41,100 +52,189 @@
 @Singleton
 @Named
 public final class DefaultTrackingFileManager
-    implements TrackingFileManager
+    implements TrackingFileManager, Service
 {
     private static final Logger LOGGER = LoggerFactory.getLogger( DefaultTrackingFileManager.class );
 
+    private static final String LOCK_PREFIX = "tracking:";
+
+    private NamedLockFactory namedLockFactory;
+
+    public DefaultTrackingFileManager()
+    {
+        // ctor for ServiceLocator
+    }
+
+    @Inject
+    public DefaultTrackingFileManager( final NamedLockFactorySelector selector )
+    {
+        this.namedLockFactory = selector.getSelectedNamedLockFactory();
+    }
+
     @Override
-    public Properties read( File file )
+    public void initService( final ServiceLocator locator )
+    {
+        NamedLockFactorySelector select = Objects.requireNonNull(
+            locator.getService( NamedLockFactorySelector.class ) );
+        this.namedLockFactory = select.getSelectedNamedLockFactory();
+    }
+
+    private String getFileKey( final File file )
     {
-        FileInputStream stream = null;
         try
         {
-            if ( !file.exists() )
+            Map<String, Object> checksums = ChecksumUtils.calc(
+                file.getCanonicalPath().getBytes( StandardCharsets.UTF_8 ),
+                Collections.singletonList( "SHA-1" ) );
+            Object checksum = checksums.get( "SHA-1" );
+            if ( checksum instanceof RuntimeException )
             {
-                return null;
+                throw (RuntimeException) checksum;
             }
-
-            stream = new FileInputStream( file );
-
-            Properties props = new Properties();
-            props.load( stream );
-
-            return props;
+            else if ( checksum instanceof Exception )
+            {
+                throw new RuntimeException( ( Exception ) checksum );
+            }
+            return LOCK_PREFIX + checksum;
         }
         catch ( IOException e )
         {
-            LOGGER.warn( "Failed to read tracking file {}", file, e );
+            throw new UncheckedIOException( e );
         }
-        finally
-        {
-            close( stream, file );
-        }
-
-        return null;
     }
 
     @Override
-    public Properties update( File file, Map<String, String> updates )
+    public Properties read( File file )
     {
-        Properties props = new Properties();
-
-        File directory = file.getParentFile();
-        if ( !directory.mkdirs() && !directory.exists() )
-        {
-            LOGGER.warn( "Failed to create parent directories for tracking file {}", file );
-            return props;
-        }
-
-        RandomAccessFile raf = null;
-        try
+        try ( NamedLock lock = namedLockFactory.getLock( getFileKey( file ) ) )

Review comment:
       on two **different** nodes using two **different** storage mounts (disks), why'd you use redisson to sync those two (unrelated) nodes at all?




-- 
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:
users@infra.apache.org