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

deploy: Remove global sync by default #2968

Merged
merged 1 commit into from
Aug 30, 2023
Merged
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
85 changes: 1 addition & 84 deletions src/libostree/ostree-sysroot-deploy.c
Original file line number Diff line number Diff line change
Expand Up @@ -1634,44 +1634,8 @@ typedef struct
{
guint64 root_syncfs_msec;
guint64 boot_syncfs_msec;
guint64 extra_syncfs_msec;
} SyncStats;

typedef struct
{
int ref_count; /* atomic */
bool success;
GMutex mutex;
GCond cond;
} SyncData;

static void
sync_data_unref (SyncData *data)
{
if (!g_atomic_int_dec_and_test (&data->ref_count))
return;
g_mutex_clear (&data->mutex);
g_cond_clear (&data->cond);
g_free (data);
}

// An asynchronous sync() call. See below.
static void *
sync_in_thread (void *ptr)
{
SyncData *syncdata = ptr;
ot_journal_print (LOG_INFO, "Starting global sync()");
sync ();
ot_journal_print (LOG_INFO, "Completed global sync()");
g_mutex_lock (&syncdata->mutex);
// Signal success
syncdata->success = true;
g_cond_broadcast (&syncdata->cond);
g_mutex_unlock (&syncdata->mutex);
sync_data_unref (syncdata);
return NULL;
}

/* First, sync the root directory as well as /var and /boot which may
* be separate mount points. Then *in addition*, do a global
* `sync()`.
Expand Down Expand Up @@ -1702,52 +1666,6 @@ full_system_sync (OstreeSysroot *self, SyncStats *out_stats, GCancellable *cance
end_msec = g_get_monotonic_time () / 1000;
out_stats->boot_syncfs_msec = (end_msec - start_msec);

/* And now out of an excess of conservativism, we still invoke
* sync(). The advantage of still using `syncfs()` above is that we
* actually get error codes out of that API, and we more clearly
* delineate what we actually want to sync in the future when this
* global sync call is removed.
*
* Due to https://bugzilla.redhat.com/show_bug.cgi?id=2003532 which is
* a bug in the interaction between Ceph and systemd, we have a capped
* timeout of 5s on the sync here. In general, we don't actually want
* to "own" flushing data on *all* mounted filesystems as part of
* our process. Again, we're only keeping the `sync` here out of
* conservatisim. We could likely just remove it and e.g. systemd
* would take care of sync as part of unmounting individual filesystems.
*/
start_msec = g_get_monotonic_time () / 1000;
if (!(self->opt_flags & OSTREE_SYSROOT_GLOBAL_OPT_SKIP_SYNC))
{
SyncData *syncdata = g_new (SyncData, 1);
syncdata->ref_count = 2; // One for us, one for thread
syncdata->success = FALSE;
g_mutex_init (&syncdata->mutex);
g_cond_init (&syncdata->cond);
g_mutex_lock (&syncdata->mutex);
GThread *worker = g_thread_new ("ostree sync", sync_in_thread, syncdata);
// Wait up to 5 seconds for sync() to finish
gint64 end_time = g_get_monotonic_time () + (5 * G_TIME_SPAN_SECOND);
while (!syncdata->success)
{
if (!g_cond_wait_until (&syncdata->cond, &syncdata->mutex, end_time))
{
ot_journal_print (LOG_INFO, "Timed out waiting for global sync()");
break;
}
}
g_mutex_unlock (&syncdata->mutex);
sync_data_unref (syncdata);
// We can't join the thread here, for two reasons. First, the whole
// point here is to avoid blocking on `sync`. The other reason is
// today there is no return value on success from `sync`, so there's
// no point to joining the thread even if we had a nonblocking mechanism
// to do so.
g_thread_unref (worker);
}
end_msec = g_get_monotonic_time () / 1000;
out_stats->extra_syncfs_msec = (end_msec - start_msec);

return TRUE;
}

Expand Down Expand Up @@ -3018,8 +2936,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, GPtrArray *n
bootloader_is_atomic ? "yes" : "no", "OSTREE_DID_BOOTSWAP=%s",
requires_new_bootversion ? "yes" : "no", "OSTREE_N_DEPLOYMENTS=%u", new_deployments->len,
"OSTREE_SYNCFS_ROOT_MSEC=%" G_GUINT64_FORMAT, syncstats.root_syncfs_msec,
"OSTREE_SYNCFS_BOOT_MSEC=%" G_GUINT64_FORMAT, syncstats.boot_syncfs_msec,
"OSTREE_SYNCFS_EXTRA_MSEC=%" G_GUINT64_FORMAT, syncstats.extra_syncfs_msec, NULL);
"OSTREE_SYNCFS_BOOT_MSEC=%" G_GUINT64_FORMAT, syncstats.boot_syncfs_msec, NULL);
_ostree_sysroot_emit_journal_msg (self, msg);
}

Expand Down
Loading