compact: add cli flag to control NoSync option

[missinglink]

The compact CLI command is very slow by default, this PR adds two new CLI flags which can be used to control the db.NoSync and db.NoFreelistSync values when creating the destination database.

Usage:

go run cmd/bbolt/main.go compact -no-sync -no-freelist-sync -o out.db in.db

Enabling these flags had a huge impact on performance, resulting in significantly expedited compact operations.

I would potentially advocate defaulting them to true, but that's a discussion for another day..

[seebs]

I'd be interested in numbers on the "huge" impact on performance. On the stuff that got me looking at fsync behavior, though, the difference was in the multiple-orders-of-magnitude range; like, the same task went from 0.01s to 5s. (This was a pathological case involving creating, in parallel, 256 boltdb databases.)

[missinglink]

I have a ~100GB database, gave up running it without this option as it took ~∞ (it worked previously running it overnight), using this PR it took some expected reasonably long amount of time, maybe like half an hour or so.

I'll measure it more scientifically for you and try to measure it again with your recent PR to get a sense of what effect completely disabling fsync has.

[missinglink]

okay, so I've run the compact operation on a smaller database which just happens to be exactly 3.0G:

The baseline measurement is the current HEAD of this repo (b18879eb6c413ffda00456687e411092c2614602):

time go run cmd/bbolt/main.go compact -o compact.bolt.db bolt.db
3021692928 -> 1807912960 bytes (gain=1.67x)

88.32s user 29.51s system 39% cpu 5:00.67 total

The second measurement is this PR with both flags set to true (a50e301a22707139039b1be40d5a8341dc06106d):

time go run cmd/bbolt/main.go compact -no-sync -no-freelist-sync -o compact.bolt.db bolt.db
3021692928 -> 1793495040 bytes (gain=1.68x)

38.59s user 4.63s system 126% cpu 34.120 total

In both cases the resultant file was 1.8G, however disabling sync operations resulted in it running ~9x faster ⚡

as an aside: I combined this PR with #291 but didn't measure any significant difference.

[missinglink]

I would actually be happy to default these flags to true if the maintainers agree?

It seems appropriate for the compact operation as it is "bulk loading data" and "you can restart the bulk load in the event of a system failure or database corruption".

I think it's natural for a user to assume that if the operation emits a fatal error that the resultant database file is corrupt and to restart the operation.

// Setting the NoSync flag will cause the database to skip fsync()
// calls after each commit. This can be useful when bulk loading data
// into a database and you can restart the bulk load in the event of
// a system failure or database corruption. Do not set this flag for
// normal use.
//
// If the package global IgnoreNoSync constant is true, this value is
// ignored.  See the comment on that constant for more details.
//
// THIS IS UNSAFE. PLEASE USE WITH CAUTION.

[xiang90]

This is safe. The downside is to scan the entire database to re-compute the free list at the startup time.

[xiang90]

(If you use this together with NoSync, it is unsafe. But the root cause is the NoSync flag, not this one)

[missinglink]

Should I remove this option?

[missinglink]

What does 'safe' mean in this context? If you mean safe from corruption due to kernel I/O errors, I'm not super worried in this context, that's unlikely right, like a power failure or someone unplugs the disk?

The compact command can either succeed or fail, if the target db is corrupt for any reason we can just fail loudly with an error.

[missinglink]

Hi, this PR has been open over a year now, is there anything preventing it from being merged?

[ahrtr]

Sorry for the late response.

A couple of points from my side:

1. Any options should be passed to bolt.Open via Options. It isn't good to change the option after bolt.Open.
2. I am supportive to expose the NoSync as long as users are happy to take the risk.
3. I am hesitate to support exposing NoFreelistSync. I am thinking it would make more sense to follow the original db. If the original db already synced free list, then the target(compacted) db should sync freelist, otherwise do not sync free list.

Anyway, please rebase this PR firstly. cc @ptabor as well.

[missinglink]

PR has been rebased, I have also removed the NoFreelistSync option.
For reference, a copy of the previous code which was discussed above can be found here.

This is good to merge

[ahrtr]

LGTM

Thank you @missinglink

[cenkalti]

IMHO the default value of --no-sync flag should be true. The original database is opened in read-only mode and there is no data loss risk if the operation is interrupted before finish.

Is there any use case for --no-sync=false case? Only one sync when closing the database should be enough for data safety.