[systemd-devel] [PATCH] [RFCv7] Add sync timer to journal server

[Lennart Poettering]

On Sun, 24.03.13 01:11, Oleksii Shevchuk (alxchk at gmail.com) wrote:

> Add option to force journal sync with fsync. Default timeout is 5min.
> Interval configured via SyncIntervalSec option at journal.conf. Synced
> journal files will be marked as OFFLINE.
> 
> Manual sync can be performed via sending SIGUSR1.

Looks mostly good.

> +int journal_file_set_online(JournalFile *f) {
> +        assert(f);
> +
> +        if (!f->writable)
> +                return -EPERM;
> +
> +        if (!(f->fd >= 0 && f->header))
> +                return -EINVAL;
> +
> +        switch(f->header->state) {
> +                case STATE_ONLINE:
> +                        return 0;
> +
> +                case STATE_OFFLINE:
> +                        f->header->state = STATE_ONLINE;
> +                        return 0;
> +
> +                default:
> +                        return -EINVAL;
> +        }
> +}

This needs an explit sync after the change. We need to make sure that
the online state has hit disk before any further changes are made to the
file. Otherwise the kernel might reorder things and flush later changes
to disk before the state change has hit disk. We really need an fsync()
or so after we changed the state.

> +int journal_file_set_offline(JournalFile *f) {
> +        assert(f);
> +
> +        if (!(f->header && f->writable && f->fd >= 0))
> +                return -EINVAL;
> +
> +        if (f->header->state != STATE_ONLINE)
> +                return 0;
> +
> +        if (fsync(f->fd))
> +                return -errno;
> +
> +        f->header->state = STATE_OFFLINE;
> +
> +        if(fsync(f->fd))
> +                return -errno;
> +
> +        return 0;
> +}

I don't think we should really care about the return value of fsync()
here... I mean, flushing things like this is nice, but if we can't,
because maybe the underlying fs doesn't do support it we shouldn't care,
should we? I'd just invoke fsync() and ignore its return value?

Otherwise I am happy.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.