svn commit: r295465 - head/usr.sbin/services_mkdb

[Stefan Esser]

Am 11.02.2016 um 10:15 schrieb David Malone:
> On Wed, Feb 10, 2016 at 09:40:45AM +0000, Stefan Esser wrote:
>> Author: se
>> Date: Wed Feb 10 09:40:45 2016
>> New Revision: 295465
>> URL: https://svnweb.freebsd.org/changeset/base/295465
>>
>> Log:
>>   Remove O_SYNC from the options passed to dbmopen().
>>   
>>   The services db is created as a temporary file that is moved over the
>>   existing file after completion. Thus there is no need to immediately
>>   flush all created db records to the temporary file.
> 
> This was changed to fix a problem with fsync not being called, which
> can leave the db files empty after a sudden reboot. O_SYNC is not
> the right fix, but I think I've found the real problem and there
> is an open phabricator report on this, which fixes the problem in
> a better way:
> 
> 	https://reviews.freebsd.org/D5186

I added the following information as a comment to D5168:

- I think that buffers are flushed in hdestroy() in hash.c, at least
  there is a comment that seems to indicate this.

- Tests showed that the _mdb commands create bitwise identical files
  with or without O_SYNC (and without the _fsync() calls you suggest
  in D5168).

- The cap_mkdb command does also use dbopen(O_SYNC) and completes in
  less than 1 second instead of more than 25 with O_SYNC removed.


BTW there is one further problem in services_mkdb:

There is a check for the existence of the temporary output file and if
a second instance of services_mkdb is started, it will terminate, but
only *after unlinking the other instance's temp output file*.

Due to the speed-up without O_SYNC the time window is much smaller now,
but I can easily provoke this even with the faster version:

# services_mkdb -qo /tmp/xx.db & services_mkdb -qo /tmp/xx.db
[2] 71869
services_mkdb: Error opening temporary database `/tmp/xx.db.tmp': File
exists
# services_mkdb-SYNC: Cannot rename `/tmp/xx.db.tmp' to `/tmp/xx.db': No
such file or directory

[2]-  Exit 1                  services_mkdb -qo /tmp/xx.db

The second process detects that the temp output file exists complains
and deletes it. When the first process completed the creation of the
db, it tries to rename the file (that has already been removed by the
second command).

A better approach might be to have the second process unlink the temp
file, but then to continue after creation of its own temp file. Then
the first process will still find that it cannot rename the temp file
to its final name, but the second one will complete its temp file and
then rename it to its final name.

Regards, STefan