Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#5920 closed enhancement (fixed)

Add retry after HTTP 503 or 504

Reported by: jpalmer Owned by: warmerdam
Priority: normal Milestone: 2.0.0
Component: default Version: svn-trunk
Severity: normal Keywords: HTTP WFS
Cc:

Description

Some WFS services are now run using reverse-proxy or load balancing services such as Nginx. In the case that a WFS request takes too long to return a result from a backend service, services such as Nginx are typically configured to return a gateway timeout error (HTTP 504). Also (maybe due to maintenance) load balancing services might return service unavailable errors (HTTP 503).

This patch makes a change to the GDAL HTTP request code to check for HTTP 503 and 504 errors and if defined by the GDAL_HTTP_MAX_RETRY and GDAL_HTTP_RETRY_DELAY config options will retry the HTTP request.

Attachments (1)

http_500_timeout.patch (7.1 KB) - added by jpalmer 7 years ago.

Download all attachments as: .zip

Change History (7)

Changed 7 years ago by jpalmer

Attachment: http_500_timeout.patch added

comment:1 Changed 7 years ago by Even Rouault

The proposed patch enables retries by default. I'm aware of GDAL deployments where a more sophisticated retry logic (with exponential retry delay plus some random value) has been implemented on the client code to deal with particular situations, so it might not be desirable to have the retry logic enabled by default in GDAL itself. Would that be OK if we change GDAL_HTTP_MAX_RETRY default to 0 ?

comment:2 Changed 7 years ago by jpalmer

Yip that sound fine to me.

I should also note that this functionality is required because large WFS paging can not be retried client side without completely restarting the whole request, which in the case of large datasets is very costly.

comment:3 Changed 7 years ago by Even Rouault

Resolution: fixed
Status: newclosed

trunk r28893 "CPLHTTPFetch(): add retry logic in case of 503 and 504 errors with the GDAL_HTTP_MAX_RETRY (default: 0) and GDAL_HTTP_RETRY_DELAY (default: 30 s) config options (derived from patch by jpalmer, #5920)"

In addition to the change from 5 to 0 retry by default, I've added doc for the options, fixed compiler warnings and also added freeing of some allocated memory. I couldn't easily test those situations, so it would be good if you could check it still works.

comment:4 Changed 7 years ago by jpalmer

Thanks! Good point on the deallocation of memory - very sloppy of me!

I will test tomorrow.

comment:5 in reply to:  4 Changed 7 years ago by Even Rouault

Replying to jpalmer:

Thanks! Good point on the deallocation of memory - very sloppy of me!

I'm not sure if any/all are really needed (depends on which code paths are taken), but hopefully they shouldn't hurt.

comment:6 Changed 6 years ago by Even Rouault

Milestone: 2.02.0.0

Milestone renamed

Note: See TracTickets for help on using tickets.