Skip to content

Commit

Permalink
Merge pull request #436 from yma96/master
Browse files Browse the repository at this point in the history 
Use more elegant way to achieve the proxy retry and extend the default job wait timeout
  • Loading branch information
@yma96
yma96 authored Aug 22, 2024
2 parents b4faac4 + 6685891 commit 1641edf
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 101 deletions.
3 changes: 2 additions & 1 deletion api/src/main/java/org/commonjava/maven/galley/config/TransportManagerConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ public class TransportManagerConfig
final long DEFAULT_WAIT_RETRY_SCALING_INCREMENT = DEFAULT_THRESHOLD_WAIT_RETRY_SIZE;
// how many times we retry depends on this.

final float DEFAULT_TIMEOUT_OVEREXTENSION_FACTOR = 1.25f;
final float DEFAULT_TIMEOUT_OVEREXTENSION_FACTOR = 2.25f;
// Use proxy retry will probably exceed one default request timeout, at least twice as much time to the first request timeout.

private final long thresholdWaitRetrySize;

Expand Down
2 changes: 1 addition & 1 deletion transports/httpclient/src/main/java/org/commonjava/maven/galley/transport/htcli/Http.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public interface Http
extends Closeable
{

CloseableHttpClient createClient( HttpLocation location, boolean isProxy )
CloseableHttpClient createClient( HttpLocation location, boolean doProxy )
throws GalleyException;

CloseableHttpClient createClient( HttpLocation location )
Expand Down
4 changes: 2 additions & 2 deletions ...sports/httpclient/src/main/java/org/commonjava/maven/galley/transport/htcli/HttpImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public CloseableHttpClient createClient( final HttpLocation location )
}

@Override
public CloseableHttpClient createClient( final HttpLocation location, final boolean isProxy )
public CloseableHttpClient createClient( final HttpLocation location, final boolean doProxy )
throws GalleyException
{
try
Expand All @@ -111,7 +111,7 @@ public CloseableHttpClient createClient( final HttpLocation location, final bool
.withUser( location.getUser() )
.withIgnoreHostnameVerification( location.isIgnoreHostnameVerification() )
.withMaxConnections( maxConnections );
if ( isProxy )
if ( doProxy )
{
logger.trace( "The location class: {}", location.getClass().getSimpleName() );
configBuilder.withProxyHost( location.getProxyHost() )
Expand Down
169 changes: 72 additions & 97 deletions ...t/src/main/java/org/commonjava/maven/galley/transport/htcli/internal/AbstractHttpJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,35 +101,80 @@ public long getTransferSize()
protected boolean executeHttp()
throws TransferException
{
boolean result = true;
int tries = 1;
boolean doProxy = false;
try
{
result = doExecuteHttp();
}
catch ( final NoHttpResponseException | ConnectTimeoutException | SocketTimeoutException e )
{
// For those are not in the iad2 tenant egress rules, will throw timeout so use the configured proxy to retry
result = executeProxyHttp();
}
catch ( final IOException e )
{
addFieldToActiveSpan( "target-error-reason", "I/O" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferLocationException( location, "Repository remote request failed for: {}. Reason: {}", e, url,
e.getMessage() );
}
catch ( TransferLocationException e )
{
addFieldToActiveSpan( "target-error-reason", "no transport" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw e;
}
catch ( final GalleyException e )
{
addFieldToActiveSpan( "target-error-reason", "unknown" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferException( "Repository remote request failed for: {}. Reason: {}", e, url,
e.getMessage() );
while ( tries > 0 )
{
tries--;
try
{
client = http.createClient( location, doProxy );
response = client.execute( request, http.createContext( location ) );

final StatusLine line = response.getStatusLine();
final int sc = line.getStatusCode();

logger.trace( "{} {} : {}", request.getMethod(), line, url );

if ( sc > 399 && sc != 404 && sc != 408 && sc != 502 && sc != 503 && sc != 504 )
{
throw new TransferLocationException( location,
"Server misconfigured or not responding normally for url %s: '%s'",
url, line );
}
else if ( !successStatuses.contains( sc ) )
{
logger.trace( "Detected failure respon se: " + sc );
success = TransferResponseUtils.handleUnsuccessfulResponse( request, response, location, url );
logger.trace( "Returning non-error failure response for code: " + sc );
return false;
}
}
catch ( final NoHttpResponseException | ConnectTimeoutException | SocketTimeoutException e )
{
// For those are not in the iad2 tenant egress rules, will throw timeout so use the configured proxy to retry
if ( tries > 0 )
{
}
else if ( !doProxy ) // never do with proxy, retry with proxy
{
tries = 1;
doProxy = true;
logger.debug( "Retry to execute with global proxy for {}", url );
}
else // already did proxy, still timeout
{
addFieldToActiveSpan( "target-error-reason", "timeout" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferTimeoutException( location, url,
"Repository remote request failed for: {}. Reason: {}", e,
url, e.getMessage() );
}
}
catch ( final IOException e )
{
addFieldToActiveSpan( "target-error-reason", "I/O" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferLocationException( location,
"Repository remote request failed for: {}. Reason: {}", e, url,
e.getMessage() );
}
catch ( TransferLocationException e )
{
addFieldToActiveSpan( "target-error-reason", "no transport" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw e;
}
catch ( final GalleyException e )
{
addFieldToActiveSpan( "target-error-reason", "unknown" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferException( "Repository remote request failed for: {}. Reason: {}", e, url,
e.getMessage() );
}
}
}
finally
{
Expand All @@ -149,76 +194,6 @@ protected boolean executeHttp()
}
}
}

return result;
}

protected boolean executeProxyHttp()
throws TransferException
{
try
{
return doExecuteHttp( true );
}
catch ( final NoHttpResponseException | ConnectTimeoutException | SocketTimeoutException e )
{
addFieldToActiveSpan( "target-error-reason", "timeout" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferTimeoutException( location, url, "Repository remote request failed for: {}. Reason: {}",
e, url, e.getMessage() );
}
catch ( final IOException e )
{
addFieldToActiveSpan( "target-error-reason", "I/O" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferLocationException( location, "Repository remote request failed for: {}. Reason: {}", e,
url, e.getMessage() );
}
catch ( TransferLocationException e )
{
addFieldToActiveSpan( "target-error-reason", "no transport" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw e;
}
catch ( final GalleyException e )
{
addFieldToActiveSpan( "target-error-reason", "unknown" );
addFieldToActiveSpan( "target-error", e.getClass().getSimpleName() );
throw new TransferException( "Repository remote request failed for: {}. Reason: {}", e, url,
e.getMessage() );
}
}

private boolean doExecuteHttp()
throws GalleyException, IOException
{
return doExecuteHttp( false );
}

private boolean doExecuteHttp( boolean isProxy )
throws GalleyException, IOException
{
client = http.createClient( location, isProxy );
response = client.execute( request, http.createContext( location ) );

final StatusLine line = response.getStatusLine();
final int sc = line.getStatusCode();

logger.trace( "{} {} : {}", request.getMethod(), line, url );

if ( sc > 399 && sc != 404 && sc != 408 && sc != 502 && sc != 503 && sc != 504 )
{
throw new TransferLocationException( location,
"Server misconfigured or not responding normally for url %s: '%s'",
url, line );
}
else if ( !successStatuses.contains( sc ) )
{
logger.trace( "Detected failure respon se: " + sc );
success = TransferResponseUtils.handleUnsuccessfulResponse( request, response, location, url );
logger.trace( "Returning non-error failure response for code: " + sc );
return false;
}
return true;
}

Expand Down

0 comments on commit 1641edf

Please sign in to comment.