Skip to content

Commit

Permalink
Merge pull request #2843 from jmarrero/retry
Browse files Browse the repository at this point in the history
ostree-fetcher-curl: handle non 404 errors as G_IO_ERROR_TIMED_OUT
  • Loading branch information
cgwalters authored Oct 18, 2023
2 parents b9e73a3 + 4fb6e6f commit 4085eee
Show file tree
Hide file tree
Showing 9 changed files with 247 additions and 17 deletions.
36 changes: 36 additions & 0 deletions man/ostree-pull.xml
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,42 @@ License along with this library. If not, see <https://www.gnu.org/licenses/>.
</para></listitem>
</varlistentry>

<varlistentry>
<term><option>--disable-retry-on-network-errors</option></term>

<listitem><para>
Do not retry when network issues happen, instead fail automatically. (Currently only affects libcurl)
</para></listitem>
</varlistentry>

<varlistentry>
<term><option>--low-speed-limit-bytes</option>=N</term>

<listitem><para>
The average transfer speed per second of a transfer during the
time set via 'low-speed-time-seconds' for libcurl to abort
(default: 1000)
</para></listitem>
</varlistentry>

<varlistentry>
<term><option>--low-speed-time-seconds</option>=N</term>

<listitem><para>
The time in number seconds that the transfer speed should be
below the 'low-speed-limit-bytes' setting for libcurl to abort
(default: 30)
</para></listitem>
</varlistentry>

<varlistentry>
<term><option>--max-outstanding-fetcher-requests</option>=N</term>

<listitem><para>
The max amount of concurrent connections allowed. (default: 8)
</para></listitem>
</varlistentry>

<varlistentry>
<term><option>--disable-verify-bindings</option></term>

Expand Down
49 changes: 40 additions & 9 deletions src/libostree/ostree-fetcher-curl.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ struct OstreeFetcher
int tmpdir_dfd;
bool force_anonymous;
char *custom_user_agent;
guint32 opt_low_speed_limit;
guint32 opt_low_speed_time;
gboolean opt_retry_all;
guint32 opt_max_outstanding_fetcher_requests;

GMainContext *mainctx;
CURLM *multi;
Expand Down Expand Up @@ -330,7 +334,13 @@ check_multi_info (OstreeFetcher *fetcher)
}
else
{
g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_FAILED,
/* When it is not a file, we want to retry the request.
* We accomplish that by using G_IO_ERROR_TIMED_OUT.
*/
gboolean opt_retry_all = req->fetcher->opt_retry_all;
int g_io_error_code
= (is_file || !opt_retry_all) ? G_IO_ERROR_FAILED : G_IO_ERROR_TIMED_OUT;
g_task_return_new_error (task, G_IO_ERROR, g_io_error_code,
"While fetching %s: [%u] %s", eff_url, curlres,
curl_easy_strerror (curlres));
_ostree_fetcher_journal_failure (req->fetcher->remote_name, eff_url,
Expand Down Expand Up @@ -664,6 +674,31 @@ _ostree_fetcher_set_proxy (OstreeFetcher *self, const char *http_proxy)
self->proxy = g_strdup (http_proxy);
}

void
_ostree_fetcher_set_low_speed_time (OstreeFetcher *self, guint32 opt_low_speed_time)
{
self->opt_low_speed_time = opt_low_speed_time;
}

void
_ostree_fetcher_set_low_speed_limit (OstreeFetcher *self, guint32 opt_low_speed_limit)
{
self->opt_low_speed_limit = opt_low_speed_limit;
}

void
_ostree_fetcher_set_retry_all (OstreeFetcher *self, gboolean opt_retry_all)
{
self->opt_retry_all = opt_retry_all;
}

void
_ostree_fetcher_set_max_outstanding_fetcher_requests (OstreeFetcher *self,
guint32 opt_max_outstanding_fetcher_requests)
{
self->opt_max_outstanding_fetcher_requests = opt_max_outstanding_fetcher_requests;
}

void
_ostree_fetcher_set_cookie_jar (OstreeFetcher *self, const char *jar_path)
{
Expand Down Expand Up @@ -912,14 +947,10 @@ initiate_next_curl_request (FetcherRequest *req, GTask *task)
g_assert_cmpint (rc, ==, CURLM_OK);
rc = curl_easy_setopt (req->easy, CURLOPT_CONNECTTIMEOUT, 30L);
g_assert_cmpint (rc, ==, CURLM_OK);
/* We used to set CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME
* here, but see https://github.com/ostreedev/ostree/issues/878#issuecomment-347228854
* basically those options don't play well with HTTP2 at the moment
* where we can have lots of outstanding requests. Further,
* we could implement that functionality at a higher level
* more consistently too.
*/

rc = curl_easy_setopt (req->easy, CURLOPT_LOW_SPEED_LIMIT, req->fetcher->opt_low_speed_limit);
g_assert_cmpint (rc, ==, CURLM_OK);
rc = curl_easy_setopt (req->easy, CURLOPT_LOW_SPEED_TIME, req->fetcher->opt_low_speed_time);
g_assert_cmpint (rc, ==, CURLM_OK);
/* closure bindings -> task */
rc = curl_easy_setopt (req->easy, CURLOPT_PRIVATE, task);
g_assert_cmpint (rc, ==, CURLM_OK);
Expand Down
39 changes: 37 additions & 2 deletions src/libostree/ostree-fetcher-soup.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ typedef struct
/* Also protected by output_stream_set_lock. */
guint64 total_downloaded;

guint32 opt_max_outstanding_fetcher_requests;

GError *oob_error;

} ThreadClosure;
Expand Down Expand Up @@ -360,6 +362,12 @@ session_thread_set_tls_database_cb (ThreadClosure *thread_closure, gpointer data
}
}

static void
session_thread_set_max_outstanding_fetcher_requests (ThreadClosure *thread_closure, gpointer data)
{
thread_closure->opt_max_outstanding_fetcher_requests = GPOINTER_TO_UINT (data);
}

static void
session_thread_set_extra_user_agent_cb (ThreadClosure *thread_closure, gpointer data)
{
Expand All @@ -377,6 +385,33 @@ session_thread_set_extra_user_agent_cb (ThreadClosure *thread_closure, gpointer
}
}

void
_ostree_fetcher_set_low_speed_time (OstreeFetcher *self, guint32 opt_low_speed_time)
{
// TODO
}

void
_ostree_fetcher_set_low_speed_limit (OstreeFetcher *self, guint32 opt_low_speed_limit)
{
// TODO
}

void
_ostree_fetcher_set_retry_all (OstreeFetcher *self, gboolean opt_retry_all)
{
// TODO
}

void
_ostree_fetcher_set_max_outstanding_fetcher_requests (OstreeFetcher *self,
guint32 opt_max_outstanding_fetcher_requests)
{
session_thread_idle_add (self->thread_closure,
session_thread_set_max_outstanding_fetcher_requests,
GUINT_TO_POINTER (opt_max_outstanding_fetcher_requests), NULL);
}

static void on_request_sent (GObject *object, GAsyncResult *result, gpointer user_data);

static void
Expand Down Expand Up @@ -511,7 +546,7 @@ ostree_fetcher_session_thread (gpointer data)
* by spreading requests across mirrors. */
gint max_conns;
g_object_get (closure->session, "max-conns-per-host", &max_conns, NULL);
if (max_conns < _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS)
if (max_conns < closure->opt_max_outstanding_fetcher_requests)
{
/* We download a lot of small objects in ostree, so this
* helps a lot. Also matches what most modern browsers do.
Expand All @@ -522,7 +557,7 @@ ostree_fetcher_session_thread (gpointer data)
* currently conservative #define SOUP_SESSION_MAX_CONNS_PER_HOST_DEFAULT 2 (as of
* 2018-02-14).
*/
max_conns = _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS;
max_conns = closure->opt_max_outstanding_fetcher_requests;
g_object_set (closure->session, "max-conns-per-host", max_conns, NULL);
}

Expand Down
28 changes: 27 additions & 1 deletion src/libostree/ostree-fetcher-soup3.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ struct OstreeFetcher
char *user_agent;

guint64 bytes_transferred;
guint32 opt_max_outstanding_fetcher_requests;
};

enum
Expand Down Expand Up @@ -380,6 +381,31 @@ _ostree_fetcher_set_extra_user_agent (OstreeFetcher *self, const char *extra_use
self->user_agent = g_strdup_printf ("%s %s", OSTREE_FETCHER_USERAGENT_STRING, extra_user_agent);
}

void
_ostree_fetcher_set_low_speed_time (OstreeFetcher *self, guint32 opt_low_speed_time)
{
// TODO
}

void
_ostree_fetcher_set_low_speed_limit (OstreeFetcher *self, guint32 opt_low_speed_limit)
{
// TODO
}

void
_ostree_fetcher_set_retry_all (OstreeFetcher *self, gboolean opt_retry_all)
{
// TODO
}

void
_ostree_fetcher_set_max_outstanding_fetcher_requests (OstreeFetcher *self,
guint32 opt_max_outstanding_fetcher_requests)
{
self->opt_max_outstanding_fetcher_requests = opt_max_outstanding_fetcher_requests;
}

static gboolean
finish_stream (FetcherRequest *request, GCancellable *cancellable, GError **error)
{
Expand Down Expand Up @@ -667,7 +693,7 @@ create_soup_session (OstreeFetcher *self)
const char *user_agent = self->user_agent ?: OSTREE_FETCHER_USERAGENT_STRING;
SoupSession *session = soup_session_new_with_options (
"user-agent", user_agent, "timeout", 60, "idle-timeout", 60, "max-conns-per-host",
_OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS, NULL);
self->opt_max_outstanding_fetcher_requests, NULL);

/* SoupContentDecoder is included in the session by default. Remove it
* if gzip compression isn't in use.
Expand Down
10 changes: 10 additions & 0 deletions src/libostree/ostree-fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ void _ostree_fetcher_set_proxy (OstreeFetcher *fetcher, const char *proxy);
void _ostree_fetcher_set_client_cert (OstreeFetcher *fetcher, const char *cert_path,
const char *key_path);

void _ostree_fetcher_set_low_speed_limit (OstreeFetcher *self, guint32 opt_low_speed_limit);

void _ostree_fetcher_set_low_speed_time (OstreeFetcher *self, guint32 opt_low_speed_time);

void _ostree_fetcher_set_retry_all (OstreeFetcher *self, gboolean opt_retry_all);

void
_ostree_fetcher_set_max_outstanding_fetcher_requests (OstreeFetcher *self,
guint32 opt_max_outstanding_fetcher_requests);

void _ostree_fetcher_set_tls_database (OstreeFetcher *self, const char *tlsdb_path);

void _ostree_fetcher_set_extra_headers (OstreeFetcher *self, GVariant *extra_headers);
Expand Down
1 change: 0 additions & 1 deletion src/libostree/ostree-repo-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ G_BEGIN_DECLS
#define _OSTREE_SUMMARY_CACHE_DIR "summaries"
#define _OSTREE_CACHE_DIR "cache"

#define _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS 8
#define _OSTREE_MAX_OUTSTANDING_DELTAPART_REQUESTS 2

/* We want some parallelism with disk writes, but we also
Expand Down
4 changes: 4 additions & 0 deletions src/libostree/ostree-repo-pull-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ typedef struct

GVariant *extra_headers;
char *append_user_agent;
guint32 low_speed_limit;
guint32 low_speed_time;
gboolean retry_all;
guint32 max_outstanding_fetcher_requests;

gboolean dry_run;
gboolean dry_run_emitted_progress;
Expand Down
Loading

0 comments on commit 4085eee

Please sign in to comment.