Hmmm, perhaps the issue isn't a sync issue, it's that DS isn't stopping in time before the systemd timeout kills it which cuold cause the issue.

Would you mind setting:

[Service]
TimeoutStopSec=600

In your .service file? You may adjust the number up as needed. I'm not sure if there is a better solution today than this.

I've already checked that. And it shuts down in like a second anyway it's a small database

I also added ps aux in ExecStopPost and verified that daemon is actually down.

Perhaps what we need is to call sync during server shutdown instead of writing it into the systemd unit file, because this could affect non-systemd systems also.

the issue of 0 byte dse.ldif has been reported several times (eg #49131) and we tried to do the best on restart to recover from an alternative. but this sounds promising to avoid the scenario.

Well it is in the doc of fsync on how exactly avoid it, just fsync the directory after doing the rename. sync() would be fine too but wouldn't that force system-wide sync which might not be desirable if user have something else running on the machine ?

Curiously, ext3/4 have mount option to do fsync automatically after rename:

   dirsync
          All directory updates within the filesystem should be done synchronously.  This affects the following
          system calls: creat, link, unlink, symlink, mkdir, rmdir, mknod and rename.

and it seems to be on by default, I couldn't repeat the issue on ext3 filesystem

This should do the trick then.

That would certainly fix that particular issue, but I have my doubts what will happen if system was shut down uncleanly during. It should fsync after close of file (from man 2 close)

   A  successful  close does not guarantee that the data has been successfully saved to disk, as the kernel uses the buffer cache to defer writes.  Typically, filesystems do not flush buffers when a
   file is closed.  If you need to be sure that the data is physically stored on the underlying disk, use fsync(2).

and then on directory itself (man 2 fsync)

Calling fsync() does not necessarily ensure that the entry in the directory containing the file has also reached disk.  For that an explicit fsync() on a file descriptor for the directory is also
   needed.

Some filesystems like ext3/4 "do that for user" when rename() is called but that is not the case in every filesystem

We don't need to call fsync on the file, we need to call it on the dir. I think that we need to do this in slapi_rename_destructive.

I'll ammend the patch then

This might be a better patch then. It calls fsync() on the configdir after rename.

ack

commit 7efc2d20f2ff6bc8f035d6e009bdd128073d5f46
To ssh://git@pagure.io/389-ds-base.git
ddf1c6d..7efc2d2 master -> master

Getting compiler warnings with this fix :-(

[root@localhost BUILD]# make install > /dev/null
../389-ds-base/ldap/servers/slapd/dse.c: In function ‘dse_write_file_nolock’:
../389-ds-base/ldap/servers/slapd/dse.c:1055:5: warning: implicit declaration of function ‘fsync’ [-Wimplicit-function-declaration]
     fsync(fp_configdir);
     ^~~~~
../389-ds-base/ldap/servers/slapd/dse.c:1056:5: warning: implicit declaration of function ‘close’ [-Wimplicit-function-declaration]
     close(fp_configdir);
     ^~~~~

Yep I just saw it myself :( I'm writing a follow up.

commit 3abcd685cfa4784715e97957b13eaef14ffe47c9
To ssh://git@pagure.io/389-ds-base.git
06c0a80..3abcd68 master -> master

One line rule

95b39e2..76feb34 389-ds-base-1.3.6 -> 389-ds-base-1.3.6

Reopen to harden missing dse case to prevent segfaults.

create_test.py will generate : id : for you. Or you can generate with:

python -c 'import uuid; print(uuid.uuid4())'

Could you please fix the docstrings? First, the '1. Create an instance' is already part of the 'setup'.
Second, we need to have all steps that are meaningful (like for 'stop the instance' too, etc.). And you test different things, so the first line should be different.

def test_restore_config(topo):
    """Check that if a dse.ldif is removed, that the server still can start
    :id: d7e61131-a34e-444d-b065-d537320958fd
    :setup: Standalone instance
    :steps:
        1. Stop the instance
        2. Delete 'dse.ldif'
        3. Start the instance
    :expectedresults:
        1. Steps 1 and 2 should be successful
        2. Server will succeed to start with restored cfg.
    """
def test_removed_config(topo):
    """Check that if a dse.ldif and backup are removed, that the server
    exits better than "segfault".
    :id: b52baa92-4590-4b31-a060-b6761471062e
    :setup: Standalone instance
    :steps:
        1. Stop the instance
        2. Delete 'dse.ldif', 'dse.ldif.bak', 'dse.ldif.startOK'
        3. Start the instance
    :expectedresults:
        1. Steps 1 and 2 should be successful
        2. Server will fail to start, but will not crash.
    """

Updated with your comments, the test parts make more sense now thanks :)

Ack from me. Pushing so this can make it into the next rhel 7.5 build

6e794a8ef..75e55e265 master -> master

d67e8aea2..3cd1ab415 389-ds-base-1.3.7 -> 389-ds-base-1.3.7

628a9272e..8dd1c60e3 389-ds-base-1.3.6 -> 389-ds-base-1.3.6

COmpiler warning

        } else {
            slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
                          "The config %s has zero length. Attempting restore ... \n", filename, rc);
            rc = PR_Delete(filename);
        }

it should be something like:

 "The config %s has zero length. Attempting restore ... (reason: %d) \n", filename, rc);

I'll fix that up with one line :)

commit d40d919646ebe6e18e4b92b2b7a04b9647285886
To ssh://git@pagure.io/389-ds-base.git
75e55e2..d40d919 master -> master

commit 0a436b7d87bcea3a71e38c5309859c759b07e2b8
To ssh://git@pagure.io/389-ds-base.git
8dd1c60..0a436b7 389-ds-base-1.3.6 -> 389-ds-base-1.3.6

commit 928e5a4809717f0393c338e22d2aa58ed6d222f4
To ssh://git@pagure.io/389-ds-base.git
3cd1ab4..928e5a4 389-ds-base-1.3.7 -> 389-ds-base-1.3.7

BZ opened with doctext.

Test case has an issue in some cases with the perl installer, that it can't remove the instance. Instead of rm dse.ldif we shoule move it. Reopen and I'll fix it asap,

@vashirov

commit 545b627ac01c712a35c59335f1b39bee8c7b30cc
To ssh://git@pagure.io/389-ds-base.git
4c4d3c1..545b627 master -> master

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2357

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.