Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use more elegant way to achieve the proxy retry and extend the default job wait timeout #436

Merged
merged 1 commit into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
Loading