lib/repo-pull: Support retrying requests on transient network errors

Allow network requests to be re-queued if they failed with a transient
error, such as a socket timeout. Retry each request up to a limit
(default: 5), and only then fail the entire pull and propagate the error
to the caller.

Add a new ostree_repo_pull_with_options() option, n-network-retries, to
control the number of retries (including setting it back to the old
default of 0, if the caller wants).

Currently, retries are not supported for FetchDeltaSuperData requests,
as they are not queued. Once they are queued, adding support for retries
should be trivial. A FIXME comment has been left for this.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Closes: #1594
Approved by: jlebon

1 files changed, 84 insertions, 10 deletions

diff --git a/src/libostree/ostree-fetcher-util.c b/src/libostree/ostree-fetcher-util.c
index aca31358..9cdb82c6 100644
--- a/src/libostree/ostree-fetcher-util.c
+++ b/src/libostree/ostree-fetcher-util.c
@@ -52,15 +52,15 @@ fetch_uri_sync_on_complete (GObject        *object,
   data->done = TRUE;
 }
 
-gboolean
-_ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher  *fetcher,
-                                            GPtrArray     *mirrorlist,
-                                            const char     *filename,
-                                            OstreeFetcherRequestFlags flags,
-                                            GBytes         **out_contents,
-                                            guint64        max_size,
-                                            GCancellable   *cancellable,
-                                            GError         **error)
+static gboolean
+_ostree_fetcher_mirrored_request_to_membuf_once  (OstreeFetcher              *fetcher,
+                                                  GPtrArray                  *mirrorlist,
+                                                  const char                 *filename,
+                                                  OstreeFetcherRequestFlags   flags,
+                                                  GBytes                    **out_contents,
+                                                  guint64                     max_size,
+                                                  GCancellable               *cancellable,
+                                                  GError                    **error)
 {
   gboolean ret = FALSE;
   g_autoptr(GMainContext) mainctx = NULL;
@@ -108,11 +108,42 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher  *fetcher,
   return ret;
 }
 
+gboolean
+_ostree_fetcher_mirrored_request_to_membuf  (OstreeFetcher              *fetcher,
+                                             GPtrArray                  *mirrorlist,
+                                             const char                 *filename,
+                                             OstreeFetcherRequestFlags   flags,
+                                             guint                       n_network_retries,
+                                             GBytes                    **out_contents,
+                                             guint64                     max_size,
+                                             GCancellable               *cancellable,
+                                             GError                    **error)
+{
+  g_autoptr(GError) local_error = NULL;
+  guint n_retries_remaining = n_network_retries;
+
+  do
+    {
+      g_clear_error (&local_error);
+      if (_ostree_fetcher_mirrored_request_to_membuf_once (fetcher, mirrorlist,
+                                                           filename, flags,
+                                                           out_contents, max_size,
+                                                           cancellable, &local_error))
+        return TRUE;
+    }
+  while (_ostree_fetcher_should_retry_request (local_error, n_retries_remaining--));
+
+  g_assert (local_error != NULL);
+  g_propagate_error (error, g_steal_pointer (&local_error));
+  return FALSE;
+}
+
 /* Helper for callers who just want to fetch single one-off URIs */
 gboolean
 _ostree_fetcher_request_uri_to_membuf (OstreeFetcher  *fetcher,
                                        OstreeFetcherURI *uri,
                                        OstreeFetcherRequestFlags flags,
+                                       guint          n_network_retries,
                                        GBytes         **out_contents,
                                        guint64        max_size,
                                        GCancellable   *cancellable,
@@ -121,7 +152,7 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher  *fetcher,
   g_autoptr(GPtrArray) mirrorlist = g_ptr_array_new ();
   g_ptr_array_add (mirrorlist, uri); /* no transfer */
   return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, NULL, flags,
-                                                     out_contents, max_size,
+                                                     n_network_retries, out_contents, max_size,
                                                      cancellable, error);
 }
 
@@ -144,3 +175,46 @@ _ostree_fetcher_journal_failure (const char *remote_name,
                    NULL);
 #endif
 }
+
+/* Check whether a particular operation should be retried. This is entirely
+ * based on how it failed (if at all) last time, and whether the operation has
+ * some retries left. The retry count is set when the operation is first
+ * created, and must be decremented by the caller. (@n_retries_remaining == 0)
+ * will always return %FALSE from this function.
+ *
+ * FIXME: In future, we may decide to use transient failures like this as a hint
+ * to prioritise other mirrors for a particular pull operation (for example). */
+gboolean
+_ostree_fetcher_should_retry_request (const GError *error,
+                                      guint         n_retries_remaining)
+{
+  if (error == NULL)
+    g_debug ("%s: error: unset, n_retries_remaining: %u",
+             G_STRFUNC, n_retries_remaining);
+  else
+    g_debug ("%s: error: %u:%u %s, n_retries_remaining: %u",
+             G_STRFUNC, error->domain, error->code, error->message,
+             n_retries_remaining);
+
+  if (error == NULL || n_retries_remaining == 0)
+    return FALSE;
+
+  /* Return TRUE for transient errors. */
+  if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT) ||
+      g_error_matches (error, G_IO_ERROR, G_IO_ERROR_HOST_NOT_FOUND) ||
+      g_error_matches (error, G_IO_ERROR, G_IO_ERROR_HOST_UNREACHABLE) ||
+#if !GLIB_CHECK_VERSION(2, 44, 0)
+      g_error_matches (error, G_IO_ERROR, G_IO_ERROR_BROKEN_PIPE) ||
+#else
+      g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CONNECTION_CLOSED) ||
+#endif
+      g_error_matches (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_NOT_FOUND) ||
+      g_error_matches (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_TEMPORARY_FAILURE))
+    {
+      g_debug ("Should retry request (remaining: %u retries), due to transient error: %s",
+               n_retries_remaining, error->message);
+      return TRUE;
+    }
+
+  return FALSE;
+}