Skip to content

Commit

Permalink
pull: use a single per-transaction syncfs instead of fsync
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@giuseppe
giuseppe committed Jan 22, 2015
1 parent 28e5e6f commit b13e13b
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 36 deletions.
101 changes: 82 additions & 19 deletions src/libostree/ostree-repo-commit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,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,
Expand Down Expand Up @@ -251,7 +252,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)
{
Expand All @@ -267,20 +268,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)
{
tmp_dest = g_strdup_printf ("tmpobject-%s.%s",
checksum,
ostree_object_type_to_string (objtype));
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;
Expand Down Expand Up @@ -474,7 +494,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)
{
Expand Down Expand Up @@ -653,15 +673,16 @@ 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;

do_commit = !have_obj;

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,
Expand Down Expand Up @@ -942,6 +963,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self,

static gboolean
cleanup_tmpdir (OstreeRepo *self,
gboolean move_tmpobject,
GCancellable *cancellable,
GError **error)
{
Expand All @@ -964,13 +986,48 @@ cleanup_tmpdir (OstreeRepo *self,
GFile *path;
guint64 mtime;
guint64 delta;
gs_free char *basename = NULL;

if (!gs_file_enumerator_iterate (enumerator, &file_info, &path,
cancellable, error))
goto out;
if (file_info == NULL)
break;

if (move_tmpobject)
{
basename = g_file_get_basename (path);
if (strncmp (basename, "tmpobject-", 10) == 0)
{
char loose_path[_OSTREE_LOOSE_PATH_MAX];
gs_free gchar *checksum = NULL;
OstreeObjectType type;
ostree_object_from_string (basename + 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 (self->tmp_dir_fd, basename,
self->objects_dir_fd, loose_path) < 0))
{
(void) unlinkat (self->tmp_dir_fd, basename, 0);
if (errno != EEXIST)
{
gs_set_error_from_errno (error, errno);
g_prefix_error (error, "Storing file '%s': ", loose_path);
goto out;
}
}
continue;
}
}
continue;

mtime = g_file_info_get_attribute_uint64 (file_info, "time::modified");
if (mtime > curtime_secs)
continue;
Expand Down Expand Up @@ -1107,7 +1164,13 @@ ostree_repo_commit_transaction (OstreeRepo *self,

g_return_val_if_fail (self->in_transaction == TRUE, FALSE);

if (!cleanup_tmpdir (self, cancellable, error))
if (syncfs (self->tmp_dir_fd) < 0)
{
g_set_error_literal (error, G_IO_ERROR, g_io_error_from_errno (errno), g_strerror (errno));
goto out;
}

if (!cleanup_tmpdir (self, TRUE, cancellable, error))
goto out;

if (self->loose_object_devino_hash)
Expand Down Expand Up @@ -1141,7 +1204,7 @@ ostree_repo_abort_transaction (OstreeRepo *self,
if (!self->in_transaction)
return TRUE;

if (!cleanup_tmpdir (self, cancellable, error))
if (!cleanup_tmpdir (self, FALSE, cancellable, error))
goto out;

if (self->loose_object_devino_hash)
Expand Down
1 change: 1 addition & 0 deletions src/libostree/ostree-repo-private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,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);

Expand Down
55 changes: 38 additions & 17 deletions src/libostree/ostree-repo.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1715,6 +1715,13 @@ load_metadata_internal (OstreeRepo *self,
cancellable, error))
goto out;

if (fd < 0)
{
sprintf (loose_path_buf, "tmpobject-%s.%s", sha256, ostree_object_type_to_string (objtype));
if (!openat_allow_noent (self->tmp_dir_fd, loose_path_buf, &fd, cancellable, error))
goto out;
}

if (fd != -1)
{
if (out_variant)
Expand Down Expand Up @@ -2111,27 +2118,52 @@ _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;
gboolean tmp_file = FALSE;

_ostree_loose_path (loose_path_buf, checksum, objtype, self->mode);

sprintf (loose_path_buf, "tmpobject-%s.%s", checksum, ostree_object_type_to_string (objtype));
do
res = fstatat (self->objects_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW);
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;
}

Expand All @@ -2143,21 +2175,10 @@ _ostree_repo_find_object (OstreeRepo *self,
GCancellable *cancellable,
GError **error)
{
gboolean ret = FALSE;
gboolean has_object;
char loose_path[_OSTREE_LOOSE_PATH_MAX];

if (!_ostree_repo_has_loose_object (self, checksum, objtype, &has_object, loose_path,
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;
return _ostree_repo_has_loose_object (self, checksum, objtype, &has_object, loose_path,
out_stored_path, cancellable, error);
}

/**
Expand Down

0 comments on commit b13e13b

Please sign in to comment.