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

ostree-fetcher-curl: handle non 404 errors as G_IO_ERROR_TIMED_OUT #2843

Merged
merged 1 commit into from
Oct 18, 2023
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
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,
cgwalters marked this conversation as resolved.
Show resolved Hide resolved
* 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
Loading