Skip to content

Commit

Permalink
[MRESOLVER-282] Drop PartialFile use (#212)
Browse files Browse the repository at this point in the history 
Drop PartialFile, clean up regarding resource handling and lessen temp file use. Downside: there is no "download resume" happening anymore.

On the other hand, from now on, resolver will download "all or nothing", so no more "partial downloads" can happen either.

HttpClientTransport is STILL ABLE to resume downloads (was the only one anyway), but due removal of PartialFile it will never resume. Later we may replace removed logic with something simpler.

---

https://issues.apache.org/jira/browse/MRESOLVER-282
  • Loading branch information
@cstamas
cstamas authored Nov 8, 2022
1 parent 76ee5e3 commit c4aa48d
Show file tree
Hide file tree
Showing 12 changed files with 213 additions and 1,041 deletions.
47 changes: 5 additions & 42 deletions ...ctor-basic/src/main/java/org/eclipse/aether/connector/basic/BasicRepositoryConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.eclipse.aether.transfer.TransferResource;
import org.eclipse.aether.transform.FileTransformer;
import org.eclipse.aether.util.ConfigUtils;
import org.eclipse.aether.util.FileUtils;
import org.eclipse.aether.util.concurrency.RunnableErrorForwarder;
import org.eclipse.aether.util.concurrency.WorkerThreadFactory;
import org.slf4j.Logger;
Expand All @@ -82,10 +83,6 @@ final class BasicRepositoryConnector

private static final String CONFIG_PROP_THREADS = "aether.connector.basic.threads";

private static final String CONFIG_PROP_RESUME = "aether.connector.resumeDownloads";

private static final String CONFIG_PROP_RESUME_THRESHOLD = "aether.connector.resumeThreshold";

private static final String CONFIG_PROP_SMART_CHECKSUMS = "aether.connector.smartChecksums";

private static final Logger LOGGER = LoggerFactory.getLogger( BasicRepositoryConnector.class );
Expand All @@ -104,8 +101,6 @@ final class BasicRepositoryConnector

private final ChecksumPolicyProvider checksumPolicyProvider;

private final PartialFile.Factory partialFileFactory;

private final int maxThreads;

private final boolean smartChecksums;
Expand Down Expand Up @@ -154,18 +149,6 @@ final class BasicRepositoryConnector
persistedChecksums =
ConfigUtils.getBoolean( session, ConfigurationProperties.DEFAULT_PERSISTED_CHECKSUMS,
ConfigurationProperties.PERSISTED_CHECKSUMS );

boolean resumeDownloads =
ConfigUtils.getBoolean( session, true, CONFIG_PROP_RESUME + '.' + repository.getId(),
CONFIG_PROP_RESUME );
long resumeThreshold =
ConfigUtils.getLong( session, 64 * 1024, CONFIG_PROP_RESUME_THRESHOLD + '.' + repository.getId(),
CONFIG_PROP_RESUME_THRESHOLD );
int requestTimeout =
ConfigUtils.getInteger( session, ConfigurationProperties.DEFAULT_REQUEST_TIMEOUT,
ConfigurationProperties.REQUEST_TIMEOUT + '.' + repository.getId(),
ConfigurationProperties.REQUEST_TIMEOUT );
partialFileFactory = new PartialFile.Factory( resumeDownloads, resumeThreshold, requestTimeout );
}

private Executor getExecutor( Collection<?> artifacts, Collection<?> metadatas )
Expand Down Expand Up @@ -430,7 +413,7 @@ protected void runTask()

class GetTaskRunner
extends TaskRunner
implements PartialFile.RemoteAccessChecker, ChecksumValidator.ChecksumFetcher
implements ChecksumValidator.ChecksumFetcher
{

private final File file;
Expand All @@ -449,13 +432,6 @@ class GetTaskRunner
checksumPolicy, providedChecksums, safe( checksumLocations ) );
}

@Override
public void checkRemoteAccess()
throws Exception
{
transporter.peek( new PeekTask( path ) );
}

@Override
public boolean fetchChecksum( URI remote, File local )
throws Exception
Expand All @@ -481,21 +457,13 @@ protected void runTask()
{
fileProcessor.mkdirs( file.getParentFile() );

PartialFile partFile = partialFileFactory.newInstance( file, this );
if ( partFile == null )
{
LOGGER.debug( "Concurrent download of {} just finished, skipping download", file );
return;
}

try
try ( FileUtils.TempFile tempFile = FileUtils.newTempFile( file.toPath() ) )
{
File tmp = partFile.getFile();
final File tmp = tempFile.getPath().toFile();
listener.setChecksumCalculator( checksumValidator.newChecksumCalculator( tmp ) );
for ( int firstTrial = 0, lastTrial = 1, trial = firstTrial; ; trial++ )
{
boolean resume = partFile.isResume() && trial <= firstTrial;
GetTask task = new GetTask( path ).setDataFile( tmp, resume ).setListener( listener );
GetTask task = new GetTask( path ).setDataFile( tmp, false ).setListener( listener );
transporter.get( task );
try
{
Expand Down Expand Up @@ -527,11 +495,6 @@ protected void runTask()
checksumValidator.commit();
}
}
finally
{
partFile.close();
checksumValidator.close();
}
}

}
Expand Down
66 changes: 12 additions & 54 deletions ...r-connector-basic/src/main/java/org/eclipse/aether/connector/basic/ChecksumValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,15 @@
import java.net.URI;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.UUID;

import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy;
import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind;
import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
import org.eclipse.aether.spi.io.FileProcessor;
import org.eclipse.aether.transfer.ChecksumFailureException;
import org.eclipse.aether.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -61,8 +60,6 @@ boolean fetchChecksum( URI remote, File local )

private final Collection<ChecksumAlgorithmFactory> checksumAlgorithmFactories;

private final Collection<File> tempFiles;

private final FileProcessor fileProcessor;

private final ChecksumFetcher checksumFetcher;
Expand All @@ -73,7 +70,7 @@ boolean fetchChecksum( URI remote, File local )

private final Collection<ChecksumLocation> checksumLocations;

private final Map<File, Object> checksumFiles;
private final Map<File, String> checksumExpectedValues;

ChecksumValidator( File dataFile,
Collection<ChecksumAlgorithmFactory> checksumAlgorithmFactories,
Expand All @@ -85,13 +82,12 @@ boolean fetchChecksum( URI remote, File local )
{
this.dataFile = dataFile;
this.checksumAlgorithmFactories = checksumAlgorithmFactories;
this.tempFiles = new HashSet<>();
this.fileProcessor = fileProcessor;
this.checksumFetcher = checksumFetcher;
this.checksumPolicy = checksumPolicy;
this.providedChecksums = providedChecksums;
this.checksumLocations = checksumLocations;
this.checksumFiles = new HashMap<>();
this.checksumExpectedValues = new HashMap<>();
}

public ChecksumCalculator newChecksumCalculator( File targetFile )
Expand Down Expand Up @@ -152,7 +148,7 @@ private boolean validateChecksums( Map<String, ?> actualChecksums, ChecksumKind

String actual = String.valueOf( calculated );
String expected = entry.getValue().toString();
checksumFiles.put( getChecksumFile( checksumAlgorithmFactory ), expected );
checksumExpectedValues.put( getChecksumFile( checksumAlgorithmFactory ), expected );

if ( !isEqualChecksum( expected, actual ) )
{
Expand Down Expand Up @@ -183,10 +179,10 @@ private boolean validateExternalChecksums( Map<String, ?> actualChecksums )
);
continue;
}
try
File checksumFile = getChecksumFile( checksumLocation.getChecksumAlgorithmFactory() );
try ( FileUtils.TempFile tempFile = FileUtils.newTempFile( checksumFile.toPath() ) )
{
File checksumFile = getChecksumFile( checksumLocation.getChecksumAlgorithmFactory() );
File tmp = createTempFile( checksumFile );
File tmp = tempFile.getPath().toFile();
try
{
if ( !checksumFetcher.fetchChecksum(
Expand All @@ -206,7 +202,7 @@ private boolean validateExternalChecksums( Map<String, ?> actualChecksums )

String actual = String.valueOf( calculated );
String expected = fileProcessor.readChecksum( tmp );
checksumFiles.put( checksumFile, tmp );
checksumExpectedValues.put( checksumFile, expected );

if ( !isEqualChecksum( expected, actual ) )
{
Expand Down Expand Up @@ -240,33 +236,10 @@ private File getChecksumFile( ChecksumAlgorithmFactory factory )
return new File( dataFile.getPath() + '.' + factory.getFileExtension() );
}

private File createTempFile( File path )
throws IOException
{
File file =
File.createTempFile( path.getName() + "-"
+ UUID.randomUUID().toString().replace( "-", "" ).substring( 0, 8 ), ".tmp", path.getParentFile() );
tempFiles.add( file );
return file;
}

private void clearTempFiles()
{
for ( File file : tempFiles )
{
if ( !file.delete() && file.exists() )
{
LOGGER.debug( "Could not delete temporary file {}", file );
}
}
tempFiles.clear();
}

public void retry()
{
checksumPolicy.onTransferRetry();
checksumFiles.clear();
clearTempFiles();
checksumExpectedValues.clear();
}

public boolean handle( ChecksumFailureException exception )
Expand All @@ -276,33 +249,18 @@ public boolean handle( ChecksumFailureException exception )

public void commit()
{
for ( Map.Entry<File, Object> entry : checksumFiles.entrySet() )
for ( Map.Entry<File, String> entry : checksumExpectedValues.entrySet() )
{
File checksumFile = entry.getKey();
Object tmp = entry.getValue();
try
{
if ( tmp instanceof File )
{
fileProcessor.move( (File) tmp, checksumFile );
tempFiles.remove( tmp );
}
else
{
fileProcessor.writeChecksum( checksumFile, String.valueOf( tmp ) );
}
fileProcessor.writeChecksum( checksumFile, entry.getValue() );
}
catch ( IOException e )
{
LOGGER.debug( "Failed to write checksum file {}", checksumFile, e );
}
}
checksumFiles.clear();
checksumExpectedValues.clear();
}

public void close()
{
clearTempFiles();
}

}
Loading

0 comments on commit c4aa48d

Please sign in to comment.