ostree-repo-pull: add options to configure retry behavior

This introduces the retry-all-network-errors option which
is enabled by default. This is a behavior change as now
ostree will retry on requests that fail except when
they fail with NOT_FOUND. It also introduces the options
low-speed-limit-bytes and low-speed-time-seconds these
map to CURL options only at the moment. Which have defaults
set following librepo:
https://github.com/rpm-software-management/librepo/blob/7c9af219abd49f8961542b7622fc82cfdaa572e3/librepo/handle.h#L90
https://github.com/rpm-software-management/librepo/blob/7c9af219abd49f8961542b7622fc82cfdaa572e3/librepo/handle.h#L96
Currently this changes only apply when using libcurl.

src/libostree/ostree-fetcher-curl.c

@@ -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;
@@ -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,
@@ -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)
 {
@@ -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);

src/libostree/ostree-fetcher-soup.c

@@ -129,6 +129,7 @@ struct OstreeFetcher
 
   GThread *session_thread;
   ThreadClosure *thread_closure;
+  guint32 opt_max_outstanding_fetcher_requests;
 };
 
 enum
@@ -377,6 +378,31 @@ 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)
+{
+  self->opt_max_outstanding_fetcher_requests = opt_max_outstanding_fetcher_requests;
+}
+
 static void on_request_sent (GObject *object, GAsyncResult *result, gpointer user_data);
 
 static void
@@ -511,7 +537,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.
@@ -522,7 +548,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);
     }
 

src/libostree/ostree-fetcher-soup3.c

@@ -84,6 +84,7 @@ struct OstreeFetcher
   char *user_agent;
 
   guint64 bytes_transferred;
+  guint32 opt_max_outstanding_fetcher_requests;
 };
 
 enum
@@ -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)
 {
@@ -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.

src/libostree/ostree-fetcher.h

@@ -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);

src/libostree/ostree-repo-private.h

@@ -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

src/libostree/ostree-repo-pull-private.h

@@ -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;

src/libostree/ostree-repo-pull.c

@@ -61,6 +61,10 @@
  * _ostree_fetcher_should_retry_request(). This is the default value for the
  * `n-network-retries` pull option. */
 #define DEFAULT_N_NETWORK_RETRIES 5
+#define OPT_LOWSPEEDLIMIT_DEFAULT 1000
+#define OPT_LOWSPEEDTIME_DEFAULT 30
+#define OPT_RETRYALL_DEFAULT TRUE
+#define OPT_OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS_DEFAULT 8
 
 typedef struct
 {
@@ -376,7 +380,7 @@ fetcher_queue_is_full (OtPullData *pull_data)
   const gboolean fetch_full
       = ((pull_data->n_outstanding_metadata_fetches + pull_data->n_outstanding_content_fetches
           + pull_data->n_outstanding_deltapart_fetches)
-         == _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS);
+         == pull_data->max_outstanding_fetcher_requests);
   const gboolean deltas_full
       = (pull_data->n_outstanding_deltapart_fetches == _OSTREE_MAX_OUTSTANDING_DELTAPART_REQUESTS);
   const gboolean writes_full = ((pull_data->n_outstanding_metadata_write_requests
@@ -2931,6 +2935,8 @@ _ostree_repo_cache_summary (OstreeRepo *self, const char *remote, GBytes *summar
 static OstreeFetcher *
 _ostree_repo_remote_new_fetcher (OstreeRepo *self, const char *remote_name, gboolean gzip,
                                  GVariant *extra_headers, const char *append_user_agent,
+                                 guint32 low_speed_limit, guint32 low_speed_time,
+                                 gboolean retry_all, guint32 max_outstanding_fetcher_requests,
                                  OstreeFetcherSecurityState *out_state, GError **error)
 {
   OstreeFetcher *fetcher = NULL;
@@ -2998,6 +3004,11 @@ _ostree_repo_remote_new_fetcher (OstreeRepo *self, const char *remote_name, gboo
       }
   }
 
+  _ostree_fetcher_set_low_speed_limit (fetcher, low_speed_limit);
+  _ostree_fetcher_set_low_speed_time (fetcher, low_speed_time);
+  _ostree_fetcher_set_retry_all (fetcher, retry_all);
+  _ostree_fetcher_set_max_outstanding_fetcher_requests (fetcher, max_outstanding_fetcher_requests);
+
   {
     g_autofree char *tls_ca_path = NULL;
 
@@ -3224,7 +3235,8 @@ reinitialize_fetcher (OtPullData *pull_data, const char *remote_name, GError **e
   g_clear_object (&pull_data->fetcher);
   pull_data->fetcher = _ostree_repo_remote_new_fetcher (
       pull_data->repo, remote_name, FALSE, pull_data->extra_headers, pull_data->append_user_agent,
-      &pull_data->fetcher_security_state, error);
+      pull_data->low_speed_limit, pull_data->low_speed_time, pull_data->retry_all,
+      pull_data->max_outstanding_fetcher_requests, &pull_data->fetcher_security_state, error);
   if (pull_data->fetcher == NULL)
     return FALSE;
 
@@ -3453,6 +3465,12 @@ all_requested_refs_have_commit (
  *   * `n-network-retries` (`u`): Number of times to retry each download on receiving
  *     a transient network error, such as a socket timeout; default is 5, 0
  *     means return errors without retrying. Since: 2018.6
+ *   * `low-speed-limit-bytes` (`u`): The average transfer speed per second of a transfer
+       during the time set via "low-speed-time-seconds" for libcurl to abort.
+ *   * `low-speed-time-seconds` (`u`): The time in number seconds that the transfer
+       speed should be below the "low-speed-limit-bytes" setting for libcurl to abort.
+ *   * `retry-all-network-errors` (`b`): Retry when network issues happen, instead of
+ * failing automatically. Currently only affects libcurl. (Default set to true)
  *   * `ref-keyring-map` (`a(sss)`): Array of (collection ID, ref name, keyring
  *     remote name) tuples specifying which remote's keyring should be used when
  *     doing GPG verification of each collection-ref. This is useful to prevent a
@@ -3509,6 +3527,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, const char *remote_name_or_base
   gboolean opt_gpg_verify_summary_set = FALSE;
   gboolean opt_collection_refs_set = FALSE;
   gboolean opt_n_network_retries_set = FALSE;
+  gboolean opt_low_speed_limit_set = FALSE;
+  gboolean opt_low_speed_time_set = FALSE;
+  gboolean opt_retry_all_set = FALSE;
+  gboolean opt_max_outstanding_fetcher_requests_set = FALSE;
   gboolean opt_ref_keyring_map_set = FALSE;
   gboolean disable_sign_verify = FALSE;
   gboolean disable_sign_verify_summary = FALSE;
@@ -3573,6 +3595,15 @@ ostree_repo_pull_with_options (OstreeRepo *self, const char *remote_name_or_base
                               &pull_data->timestamp_check_from_rev);
       (void)g_variant_lookup (options, "max-metadata-size", "t", &pull_data->max_metadata_size);
       (void)g_variant_lookup (options, "append-user-agent", "s", &pull_data->append_user_agent);
+      opt_low_speed_limit_set
+          = g_variant_lookup (options, "low-speed-limit-bytes", "u", &pull_data->low_speed_limit);
+      opt_low_speed_time_set
+          = g_variant_lookup (options, "low-speed-time-seconds", "u", &pull_data->low_speed_time);
+      opt_retry_all_set
+          = g_variant_lookup (options, "retry-all-network-errors", "b", &pull_data->retry_all);
+      opt_max_outstanding_fetcher_requests_set
+          = g_variant_lookup (options, "max-outstanding-fetcher-requests", "b",
+                              &pull_data->max_outstanding_fetcher_requests);
       opt_n_network_retries_set
           = g_variant_lookup (options, "n-network-retries", "u", &pull_data->n_network_retries);
       opt_ref_keyring_map_set
@@ -3647,6 +3678,15 @@ ostree_repo_pull_with_options (OstreeRepo *self, const char *remote_name_or_base
 
   if (!opt_n_network_retries_set)
     pull_data->n_network_retries = DEFAULT_N_NETWORK_RETRIES;
+  if (!opt_low_speed_limit_set)
+    pull_data->low_speed_limit = OPT_LOWSPEEDLIMIT_DEFAULT;
+  if (!opt_low_speed_time_set)
+    pull_data->low_speed_time = OPT_LOWSPEEDTIME_DEFAULT;
+  if (!opt_retry_all_set)
+    pull_data->retry_all = OPT_RETRYALL_DEFAULT;
+  if (!opt_max_outstanding_fetcher_requests_set)
+    pull_data->max_outstanding_fetcher_requests
+        = OPT_OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS_DEFAULT;
 
   pull_data->repo = self;
   pull_data->progress = progress;
@@ -5696,7 +5736,7 @@ find_remotes_cb (GObject *obj, GAsyncResult *async_result, gpointer user_data)
                 goto error;
 
               fetcher = _ostree_repo_remote_new_fetcher (self, result->remote->name, TRUE, NULL,
-                                                         NULL, NULL, &error);
+                                                         NULL, 0, 0, TRUE, 0, NULL, &error);
               if (fetcher == NULL)
                 goto error;
 
@@ -6310,6 +6350,10 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, const char *nam
   g_autoptr (GVariant) extra_headers = NULL;
   g_autoptr (GPtrArray) mirrorlist = NULL;
   const char *append_user_agent = NULL;
+  guint32 low_speed_limit = OPT_LOWSPEEDLIMIT_DEFAULT;
+  guint32 low_speed_time = OPT_LOWSPEEDTIME_DEFAULT;
+  gboolean retry_all = OPT_RETRYALL_DEFAULT;
+  guint32 max_outstanding_fetcher_requests = OPT_OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS_DEFAULT;
   guint n_network_retries = DEFAULT_N_NETWORK_RETRIES;
   gboolean summary_sig_not_modified = FALSE;
   g_autofree char *summary_sig_if_none_match = NULL;
@@ -6334,6 +6378,11 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, const char *nam
       (void)g_variant_lookup (options, "http-headers", "@a(ss)", &extra_headers);
       (void)g_variant_lookup (options, "append-user-agent", "&s", &append_user_agent);
       (void)g_variant_lookup (options, "n-network-retries", "u", &n_network_retries);
+      (void)g_variant_lookup (options, "low-speed-limit-bytes", "u", &low_speed_limit);
+      (void)g_variant_lookup (options, "low-speed-time-seconds", "u", &low_speed_time);
+      (void)g_variant_lookup (options, "retry-all-network-errors", "b", &retry_all);
+      (void)g_variant_lookup (options, "max-outstanding-fetcher-requests", "b",
+                              &max_outstanding_fetcher_requests);
     }
 
   if (!ostree_repo_remote_get_gpg_verify_summary (self, name, &gpg_verify_summary, error))
@@ -6346,7 +6395,8 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, const char *nam
   (void)mainctx; // Used for autocleanup
 
   fetcher = _ostree_repo_remote_new_fetcher (self, name, TRUE, extra_headers, append_user_agent,
-                                             NULL, error);
+                                             low_speed_limit, low_speed_time, retry_all,
+                                             max_outstanding_fetcher_requests, NULL, error);
   if (fetcher == NULL)
     return FALSE;