pull: use a single per-transaction syncfs instead of fsync

Do not write directly to objects/ but maintain pulled files under tmp/
with a "tmpobject-$CHECKSUM.$OBJTYPE" name until they are syncfs'ed to
disk.

Move them under objects/ at ostree_repo_commit_transaction cleanup
time.

Before (test done on a local network):

$ LANG=C sudo time ./ostree --repo=repo pull origin master

0 metadata, 3 content objects fetched; 83820 KiB; 4 delta parts
fetched, transferred in 417 seconds
16.42user 6.73system 6:57.19elapsed 5%CPU (0avgtext+0avgdata
248428maxresident)k
24inputs+794472outputs (0major+233968minor)pagefaults 0swaps

After:

$ LANG=C sudo time ./ostree --repo=repo pull origin master

0 metadata, 3 content objects fetched; 83820 KiB; 4 delta parts
fetched, transferred in 9 seconds
14.70user 2.87system 0:09.99elapsed 175%CPU (0avgtext+0avgdata
256168maxresident)k
0inputs+794472outputs (0major+164333minor)pagefaults 0swaps

https://bugzilla.gnome.org/show_bug.cgi?id=728065

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

src/libostree/ostree-repo-commit.c

@@ -35,6 +35,7 @@
 #include "ostree-mutable-tree.h"
 #include "ostree-varint.h"
 #include <attr/xattr.h>
+#include <glib/gprintf.h>
 
 gboolean
 _ostree_repo_ensure_loose_objdir_at (int             dfd,
@@ -59,6 +60,19 @@ _ostree_repo_ensure_loose_objdir_at (int             dfd,
   return TRUE;
 }
 
+void
+_ostree_repo_get_tmpobject_path (OstreeRepo       *repo,
+                                 char             *output,
+                                 const char       *checksum,
+                                 OstreeObjectType  objtype)
+{
+  g_sprintf (output,
+             "%s/tmpobject-%s.%s",
+             repo->boot_id,
+             checksum,
+             ostree_object_type_to_string (objtype));
+}
+
 static GVariant *
 create_file_metadata (GFileInfo *file_info,
                       GVariant     *xattrs)
@@ -108,6 +122,7 @@ write_file_metadata_to_xattr (int fd,
 
 static gboolean
 commit_loose_object_trusted (OstreeRepo        *self,
+                             const char        *checksum,
                              OstreeObjectType   objtype,
                              const char        *loose_path,
                              GFile             *temp_file,
@@ -251,7 +266,7 @@ commit_loose_object_trusted (OstreeRepo        *self,
       /* Ensure that in case of a power cut, these files have the data we
        * want.   See http://lwn.net/Articles/322823/
        */
-      if (!self->disable_fsync)
+      if (!self->in_transaction && !self->disable_fsync)
         {
           if (fsync (fd) == -1)
             {
@@ -267,20 +282,39 @@ commit_loose_object_trusted (OstreeRepo        *self,
   if (!_ostree_repo_ensure_loose_objdir_at (self->objects_dir_fd, loose_path,
                                             cancellable, error))
     goto out;
-  
-  if (G_UNLIKELY (renameat (self->tmp_dir_fd, temp_filename,
-                            self->objects_dir_fd, loose_path) == -1))
-    {
-      if (errno != EEXIST)
-        {
-          gs_set_error_from_errno (error, errno);
-          g_prefix_error (error, "Storing file '%s': ", temp_filename);
-          goto out;
-        }
-      else
-        (void) unlinkat (self->tmp_dir_fd, temp_filename, 0);
-    }
 
+  {
+    gs_free gchar *tmp_dest = NULL;
+    int dir;
+    const char *dest;
+
+    if (self->in_transaction)
+      {
+        char tmpbuf[_OSTREE_LOOSE_PATH_MAX];
+        _ostree_repo_get_tmpobject_path (self, tmpbuf, checksum, objtype);
+        tmp_dest = g_strdup (tmpbuf);
+        dir = self->tmp_dir_fd;
+        dest = tmp_dest;
+      }
+    else
+      {
+        dir = self->objects_dir_fd;
+        dest = loose_path;
+      }
+
+    if (G_UNLIKELY (renameat (self->tmp_dir_fd, temp_filename,
+                              dir, dest) == -1))
+      {
+        if (errno != EEXIST)
+          {
+            gs_set_error_from_errno (error, errno);
+            g_prefix_error (error, "Storing file '%s': ", temp_filename);
+            goto out;
+          }
+        else
+          (void) unlinkat (self->tmp_dir_fd, temp_filename, 0);
+      }
+  }
   ret = TRUE;
  out:
   return ret;
@@ -474,7 +508,7 @@ write_object (OstreeRepo         *self,
     {
       if (!_ostree_repo_has_loose_object (self, expected_checksum, objtype,
                                           &have_obj, loose_objpath,
-                                          cancellable, error))
+                                          NULL, cancellable, error))
         goto out;
       if (have_obj)
         {
@@ -655,7 +689,7 @@ write_object (OstreeRepo         *self,
     }
 
   if (!_ostree_repo_has_loose_object (self, actual_checksum, objtype,
-                                      &have_obj, loose_objpath,
+                                      &have_obj, loose_objpath, NULL,
                                       cancellable, error))
     goto out;
           
@@ -663,7 +697,8 @@ write_object (OstreeRepo         *self,
 
   if (do_commit)
     {
-      if (!commit_loose_object_trusted (self, objtype, loose_objpath,
+      if (!commit_loose_object_trusted (self, actual_checksum,
+                                        objtype, loose_objpath,
                                         temp_file, temp_filename,
                                         object_is_symlink, file_info,
                                         xattrs, temp_out,
@@ -930,6 +965,17 @@ ostree_repo_prepare_transaction (OstreeRepo     *self,
       if (!ot_gfile_ensure_unlinked (self->transaction_lock_path, cancellable, error))
         goto out;
     }
+
+  if (mkdirat (self->tmp_dir_fd, self->boot_id, 0777) == -1)
+    {
+      int errsv = errno;
+      if (G_UNLIKELY (errsv != EEXIST))
+        {
+          gs_set_error_from_errno (error, errsv);
+          goto out;
+        }
+    }
+
   transaction_str = g_strdup_printf ("pid=%llu", (unsigned long long) getpid ());
   if (!g_file_make_symbolic_link (self->transaction_lock_path, transaction_str,
                                   cancellable, error))
@@ -942,6 +988,65 @@ ostree_repo_prepare_transaction (OstreeRepo     *self,
   return ret;
 }
 
+static gboolean
+rename_pending_loose_objects (OstreeRepo        *self,
+                              GCancellable      *cancellable,
+                              GError           **error)
+{
+  gboolean ret = FALSE;
+  gs_dirfd_iterator_cleanup GSDirFdIterator child_dfd_iter = { 0, };
+
+  if (!gs_dirfd_iterator_init_at (self->tmp_dir_fd, self->boot_id, FALSE, &child_dfd_iter, error))
+    goto out;
+
+  while (TRUE)
+    {
+      struct dirent *out_dent;
+
+      if (!gs_dirfd_iterator_next_dent (&child_dfd_iter, &out_dent, cancellable, error))
+        goto out;
+
+      if (out_dent == NULL)
+        break;
+
+      if (strncmp (out_dent->d_name, "tmpobject-", 10) == 0)
+        {
+          char loose_path[_OSTREE_LOOSE_PATH_MAX];
+          gs_free gchar *checksum = NULL;
+          OstreeObjectType type;
+          ostree_object_from_string (out_dent->d_name + 10,
+                                     &checksum,
+                                     &type);
+
+          _ostree_loose_path (loose_path, checksum, type, self->mode);
+
+          if (!_ostree_repo_ensure_loose_objdir_at (self->objects_dir_fd, loose_path,
+                                                    cancellable, error))
+            goto out;
+
+          if (G_UNLIKELY (renameat (child_dfd_iter.fd, out_dent->d_name,
+                                    self->objects_dir_fd, loose_path) < 0))
+            {
+              (void) unlinkat (self->tmp_dir_fd, out_dent->d_name, 0);
+              if (errno != EEXIST)
+                {
+                  gs_set_error_from_errno (error, errno);
+                  g_prefix_error (error, "Storing file '%s': ", loose_path);
+                  goto out;
+                }
+            }
+          continue;
+        }
+    }
+
+  if (!gs_shutil_rm_rf_at (self->tmp_dir_fd, self->boot_id, cancellable, error))
+    goto out;
+
+  ret = TRUE;
+ out:
+  return ret;
+}
+
 static gboolean
 cleanup_tmpdir (OstreeRepo        *self,
                 GCancellable      *cancellable,
@@ -1109,6 +1214,15 @@ ostree_repo_commit_transaction (OstreeRepo                  *self,
 
   g_return_val_if_fail (self->in_transaction == TRUE, FALSE);
 
+  if (syncfs (self->tmp_dir_fd) < 0)
+    {
+      gs_set_error_from_errno (error, errno);
+      goto out;
+    }
+
+  if (! rename_pending_loose_objects (self, cancellable, error))
+    goto out;
+
   if (!cleanup_tmpdir (self, cancellable, error))
     goto out;
 

src/libostree/ostree-repo-private.h

@@ -34,6 +34,8 @@ G_BEGIN_DECLS
 struct OstreeRepo {
   GObject parent;
 
+  char *boot_id;
+
   GFile *repodir;
   GFile *tmp_dir;
   int    tmp_dir_fd;
@@ -82,6 +84,11 @@ _ostree_repo_ensure_loose_objdir_at (int             dfd,
                                      const char     *loose_path,
                                      GCancellable   *cancellable,
                                      GError        **error);
+void
+_ostree_repo_get_tmpobject_path (OstreeRepo       *repo,
+                                 char             *output,
+                                 const char       *checksum,
+                                 OstreeObjectType  objtype);
 
 gboolean
 _ostree_repo_find_object (OstreeRepo           *self,
@@ -101,6 +108,7 @@ _ostree_repo_has_loose_object (OstreeRepo           *self,
                                OstreeObjectType      objtype,
                                gboolean             *out_is_stored,
                                char                 *loose_path_buf,
+                               GFile               **out_stored_path,
                                GCancellable         *cancellable,
                                GError             **error);
 

src/libostree/ostree-repo.c

@@ -326,6 +326,7 @@ ostree_repo_finalize (GObject *object)
 
   g_clear_object (&self->parent_repo);
 
+  g_free (self->boot_id);
   g_clear_object (&self->repodir);
   g_clear_object (&self->tmp_dir);
   if (self->tmp_dir_fd)
@@ -1277,6 +1278,16 @@ ostree_repo_open (OstreeRepo    *self,
   if (self->inited)
     return TRUE;
 
+  /* We use a per-boot identifier to keep track of which file contents
+   * possibly haven't been sync'd to disk.
+   */
+  if (!g_file_get_contents ("/proc/sys/kernel/random/boot_id",
+                           &self->boot_id,
+                           NULL,
+                           error))
+    goto out;
+  g_strdelimit (self->boot_id, "\n", '\0');
+
   if (!gs_file_open_dir_fd (self->objects_dir, &self->objects_dir_fd, cancellable, error))
     {
       g_prefix_error (error, "Reading objects/ directory: ");
@@ -1715,6 +1726,13 @@ load_metadata_internal (OstreeRepo       *self,
                            cancellable, error))
     goto out;
 
+  if (self->in_transaction && fd < 0)
+    {
+      _ostree_repo_get_tmpobject_path (self, loose_path_buf, sha256, objtype);
+      if (!openat_allow_noent (self->tmp_dir_fd, loose_path_buf, &fd, cancellable, error))
+        goto out;
+    }
+
   if (fd != -1)
     {
       if (out_variant)
@@ -2111,27 +2129,55 @@ _ostree_repo_has_loose_object (OstreeRepo           *self,
                                OstreeObjectType      objtype,
                                gboolean             *out_is_stored,
                                char                 *loose_path_buf,
+                               GFile               **out_stored_path,
                                GCancellable         *cancellable,
                                GError             **error)
 {
   gboolean ret = FALSE;
   struct stat stbuf;
-  int res;
+  int res = -1;
+  gboolean tmp_file = FALSE;
 
-  _ostree_loose_path (loose_path_buf, checksum, objtype, self->mode);
+  if (self->in_transaction)
-
-  do
-    res = fstatat (self->objects_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW);
-  while (G_UNLIKELY (res == -1 && errno == EINTR));
-  if (res == -1 && errno != ENOENT)
     {
-      gs_set_error_from_errno (error, errno);
+      _ostree_repo_get_tmpobject_path (self, loose_path_buf, checksum, objtype);
-      goto out;
+      do
+        res = fstatat (self->tmp_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW);
+      while (G_UNLIKELY (res == -1 && errno == EINTR));
+      if (res == -1 && errno != ENOENT)
+        {
+          gs_set_error_from_errno (error, errno);
+          goto out;
+        }
+    }
+
+  if (res == 0)
+    tmp_file = TRUE;
+  else
+    {
+      _ostree_loose_path (loose_path_buf, checksum, objtype, self->mode);
+
+      do
+        res = fstatat (self->objects_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW);
+      while (G_UNLIKELY (res == -1 && errno == EINTR));
+      if (res == -1 && errno != ENOENT)
+        {
+          gs_set_error_from_errno (error, errno);
+          goto out;
+        }
     }
 
   ret = TRUE;
   *out_is_stored = (res != -1);
- out:
+
+  if (out_stored_path)
+    {
+      if (res != -1)
+        *out_stored_path = g_file_resolve_relative_path (tmp_file ? self->tmp_dir : self->objects_dir, loose_path_buf);
+      else
+        *out_stored_path = NULL;
+    }
+out:
   return ret;
 }
 
@@ -2143,21 +2189,10 @@ _ostree_repo_find_object (OstreeRepo           *self,
                           GCancellable         *cancellable,
                           GError             **error)
 {
-  gboolean ret = FALSE;
   gboolean has_object;
   char loose_path[_OSTREE_LOOSE_PATH_MAX];
-
+  return _ostree_repo_has_loose_object (self, checksum, objtype, &has_object, loose_path,
-  if (!_ostree_repo_has_loose_object (self, checksum, objtype, &has_object, loose_path, 
+                                        out_stored_path, cancellable, error);
-                                      cancellable, error))
-    goto out;
-
-  ret = TRUE;
-  if (has_object)
-    *out_stored_path = g_file_resolve_relative_path (self->objects_dir, loose_path);
-  else
-    *out_stored_path = NULL;
-out:
-  return ret;
 }
 
 /**