pthread_cancel doesn't really work properly. It was only being used in
bees to bring threads to a stop if the BeesContext is destroyed early.
It is frequently implicated in core dump reports because of the fragility
of the C++ iostream / C stdio / library infrastructure, particularly
surrounding upgrades on the host running bees. The pthread_cancel call
itself often simply fails even when it doesn't call terminate().
Defer creation of the status and progress threads until after the
BeesContext::start method is invoked. At that point, the existing
ask-threads-nicely-to-stop code is up and running, and normal condvars
can be used to bring bees to a stop, without having to resort to
pthread_cancel.
Since we're deleting half of the BeesContext constructor in this change,
let's remove the other half too, and put an end to the deprecated support
for multiple BeesContexts sharing a process. It's still possible to run
multiple BeesContexts, but they will not share a FD cache. This will
allow the FD cache's keys to become smaller and hopefully save some
memory later on.
Fixes: #171
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
C99's "{ 0 }" notation for filling in a struct with all zeros was not
included in the C++11 standard, so gcc doesn't implement it and neither
does clang.
gcc does (did?) have issues with warnings on the same code in C99,
complaining about uninitialized struct members when "{0}" explicitly
initializes every member to a zero value. These issues don't apply in
the C++ code where NTOA_TABLE_ENTRY_END is used.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Get rid of an assert in bits_ntoa. Throw an exception instead.
Fix hex formatting (adding "0x" before a decimal number is not
the correct way to format hex strings).
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The kernel from such an old distro version likely has several unfixed
bugs. Better not to support it at all.
Users who can upgrade the kernel are probably also sophisticated enough
to fix the build issues too.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The weird things distros do to the path where uuid.h gets installed
have broken bees builds for the last time.
We were only using uuid to support a legacy feature that was removed
over four years ago.
Hypothetical users who are upgrading directly from bees v0.1 should
probably restart all the crawlers anyway--there were bugs. Also, if any
such users exist, I respect their tremendous patience with the horrible
performance all these years--bees got about 30x faster since v0.1.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The slow backrefs performance improvement is confirmed by reports from
multiple users:
* Me (5.4.60 + backref patches, 5.7 to 5.11)
* https://github.com/Zygo/bees/issues/161 (5.8)
* https://github.com/Zygo/bees/issues/162 (5.8)
* IRC user S0rin (5.4.88 + backref patches)
The issue still exists, but at a significantly reduced scale: now about
2 ms of CPU per ref on a fast machine.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The Linux kernel's btrfs headers are better than the libbtrfs-dev headers:
- the libbtrfs-dev headers have C++ language compatibility issues
- upstream version in Linux kernel is more accurate and up to date
- macros in libbtrfs-dev's ctree.h hide information that would
enable bees to perform runtime buffer length checking
- enum types whose presence cannot be detected with #ifdef
When accessing members of metadata items from the filesystem, we want
to verify that the member we are accessing is within the boundaries of
the item that was retrieved; otherwise, a memory access violation may
occur or garbage may be returned to the caller. A simple C++ template,
given a pointer to a structure member and a buffer, can determine that
the buffer contains enough bytes to safely access a struct member.
This was implemented back in 2016, but left unused due to ctree.h issues.
Some btrfs metadata structures have variable length despite using a
fixed-size in-memory structure. The members that appear earliest in
the structure contain information about which following members of the
structure are used. The item stored in the filesystem is truncated after
the last used member, and all following members must not be accessed.
'btrfs_stack_*' accessor macros obscure the memory boundaries of the
members they access, which makes it impossible for a C++ template to
verify the memory access. If the template checks the length of the
entire structure, it will find an access violation for variable-length
metadata items because the item is rarely large enough for the entire
structure.
Get rid of all the libbtrfs-dev accessor macros and reimplement them
with the necessary buffer length checks.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Apparently it is missing in newer Linux headers, making
builds fail. We don't need it, so remove it.
Closes: #160
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Make these workarounds configurable in src/bees.h instead of #if 0
code blocks. Someday we'll make the constants in bees.h configurable
through a file or similar.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Spanner<Iterator> turns a pair of pointers into a sequence container
with several of vector's methods.
A partial specialization of make_spanner is provided which uses
shared_ptr as the beginning of the range. Some of the Spanner code
is a questionable hack in support of this.
C++20 has ranges and span, but neither is worth moving the minimum
C++ standard forward.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
When we are using non-copying containers, we can't call resize() on them.
get_struct_ptr is essentially a pointer cast, so we will end up with a
pointer to a struct that extends beyond the boundaries of the container.
As long as the btrfs metadata is not corrupted, we should not have too
many problems.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Use uint8_t when we mean uint8_t, i.e. vector<uint8_t> instead of
vector<char>.
Add a template parameter instead of vector so we can swap in a
non-copying data type.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Define a local copy of the header that has fields for the csum type
and length, so we can build in places that haven't caught up to kernel
5.5 headers yet.
The reason why the csum type and length are not unconditionally filled
in eludes me. csum_length is necessarily non-zero, and the cost of
the conditional is worse than the cost of the copy, so the whole flags
dance is a WTF...but it's part of the kernel API now, so it's too late
to NAK it.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Rewrite Fd using a much simpler named resource template class with
a more straightforward derivation strategy.
Behavior change: we no longer throw an exception while calling get_fd()
on a closed Fd. This does not seem to bother any current callers except
for the tests.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
fiewalk and fiemap depend on a lot of crucible, and incremental builds
fail hard without proper dependency tracking.
All binaries must be rebuilt when makeflags changes. This dependency
exists already in lib and test, but src was missing.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
NamedPtr provides reference-counted handles to named objects. The object
is created the first time the associated name is used, and stored under
the associated name until the last handle is destroyed. NamedPtr may
itself be destroyed while handles are still active.
This template is intended to replace ResourceHandle with a more general
and less invasive implementation.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Use a single static variable located in the library, instead of
having a separate one for each compilation unit.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
std::list and std::map both have stable iterators, and list has the
splice() method, so we don't need a hand-rolled double-linked list here.
Coalesce insert() and operator() into a single function.
Drop the unused prune() method.
Move destructor calls for cached objects out from under the cache lock.
Closing a lot of files at once is already expensive, might as well not
stop the world while we do it.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Due to a missing dependency, tests are not rebuilt when the library
changes, so tests return false results after library source changes.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
If we create an identical .version.cc then don't bother keeping it.
This prevents libcrucible from rebuilding if there are no other changes,
which in turn prevents all the binaries from rebuilding unconditionally.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Some versions of linux-libc header files define a macro named 'crc32c'.
We want to use that name too, so #undef it.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Now that tempfiles are using pool checkin functions to control their
size, we don't need a size limit in realign().
We keep the limit in make_copy because it's a sanity check against
letting a multi-terabyte copy operation slip through.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Get rid of the thread-local TempFiles and use Pool instead. This
eliminates a potential FD leak when the loadavg governor repeatedly
creates and destroys threads.
With the old per-thread TempFiles, we were guaranteed to have exclusive
ownership of the TempFile object within the current thread. Pool is
somewhat stricter: it only guarantees ownership while the checked-out
Handle exists. Adjust the users of TempFile objects to ensure they hold
the Handle object until they are finished using the TempFile.
It appears that maintaining large, heavily-reflinked, long-lived temporary
files costs more than truncating after every use: btrfs has to write
multiple references to the temporary file's extents, then some commits
later, remove references as the temporary file is deleted or truncated.
Using the temporary file in a dedupe operation flushes the data to disk,
so nothing is saved by pretending that there is writeback pipelining and
trying to avoid flushes in truncate. Pool provides usage tracking and
a checkin callback, so use it to truncate the temporary file immediately
after every use.
Redesign TempFile so that every instance creates exactly one Fd which
persists over the lifetime of the TempFile object. Provide a reset()
method which resets the file back to the initial state and call it from
the Pool checkin callback. This makes TempFile's lifetime equivalent to
its Fd's lifetime, which simplifies interactions with FdCache and Roots.
This change means we can now blacklist temporary files without having
an effective memory leak, so do that. We also have a reason to ever
remove something from the blacklist, so add a method for that too.
In order to move to extent-centric addressing, we need to be able to
reliably open temporary files by root and inode number. Previously we
would place TempFile fd's into the cache with insert_root_ino, but the
cache would be cleared periodically, and it would not be possible to
reopen temporary files after that happened. Now that the TempFile's
lifetime is the same as the TempFile Fd's lifetime, we can have TempFile
manage a separate FileId -> Fd map in Roots which is unaffected by the
periodic cache clearing. BeesRoots::open_root_ino_nocache will check
this map before attempting to open the file via btrfs root+ino lookup,
and return it through the cache as if Roots had opened the file via btrfs.
Hold a reference to BeesRoots in BeesTempFile because the usual way
to get such a reference now throws an exception in BeesTempFile's
destructor.
These changes make method BeesTempFile::create() and all methods named
insert_root_ino unnecessary, so delete them.
We construct and destroy TempFiles much less often now, so make their
constructor and destructor more informative.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Pool is a place to store shared_ptrs to generated objects (T) that are
too expensive to create and destroy between individual uses, such as
temporary files. Objects in a Pool have no distinct identity
(contrast with Cache or NamedPtr).
Users of the Pool invoke the Pool function call overload and "check out"
a shared_ptr<T> for a T object from the Pool. When the last referencing
shared_otr<T> is destroyed, the T object is "checked in" to the Pool.
Each call of the Pool function overload checks out a shared_ptr<T> to a T
object that is not currently referenced by any other public shared_ptr<T>.
If there are no existing T objects in the Pool, a new T is constructed
by calling the generator function.
The clear() method destroys all checked in T objects owned by the Pool
at the time the method is called. T objects that are checked out are
not affected by clear(), and they will be stored in the Pool when they
are checked in.
If the checkout function is provided, it is called on a shared_ptr<T>
during checkout, before returning to the caller.
If the checkin function is provided, it is called on a shared_ptr<T>
before returning it to the Pool. The checkin function must not throw
exceptions.
The Pool may be destroyed while T objects are checked out of the Pool.
In that case, when the T objects are checked in, the T object is
immediately destroyed without calling the checkin function.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
A prealloc extent reference can be deduped immediately and asynchronously.
There is no need to slow down extent scanning to do it.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
I was never able to prove a connection between fsync() and deadlock bugs.
There were too many deadlock bugs to be able to isolate a bug that is
triggered specifically by fsync.
Update the comment (which has been unchanged since kernel 4.14). We still
may want to do fsync() on temporary files someday, but there's a full
internal API rewrite between here and there.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Perf blames this operator for >1% of instructions with -O2, and
70% of instructions without -O2.
Let the compiler inline the function.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The requested size may not match the final size of the container,
so consistently use the container's size after prepare(), not the
requested size.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Number of items should be low enough that we don't have too many stale
items, but high enough to amortize system call overhead to a reasonable
ratio.
Number of bytes should be constant: one worst-case metadata page (the
btrfs limit is 64K, though 16K is much more common) so that we always
have enough space for one worst-case item; otherwise, we get EOVERFLOW
if we set the number of items too low and there's a big item in the tree,
and we can't make further progress.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
There are lots of ways the search can fail, but it's hard to pick one
without knowing the parameters.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
It's a pain to read, edit, and format large blocks of text in C++ code,
so rip the usage message out of bees.cc and put it in a plain text file.
Use a minimal translator to convert it into a C string.
While we're here, remove the multiple roots feature from the command
line synopsis, as we don't really support it any more. Also clarify
that "id 5" is "subvol id 5", and describe in one sentence what
workaround-btrfs-send does.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Silence the unused variable warning. The compiler is correct, but we
may implement line-level debug at some point in the future, so we
want to keep the member and parameters.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Remove unused function getenv_or_die. All of our environment variable
parameters are optional or have default values.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Get rid of unused template instantiation.
Drop the unused realtime signals from the ntoa table. If in the future
we really need to solve clang's issue with them, we'll address it then.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
A long time ago, when bees used dedicated threads to scan each subvol, the
calculation of the "dedup_unique_bytes" statistic was still wrong.
This stat can only be calculated when dedupe runs on extent data items
instead of extent reference items. Remove the stat variable until
that happens.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
There was a 4th tree mod log crash that showed up in testing. It can
be reproduced or eliminated by applying or reverting d2311e698578
("btrfs: relocation: Delay reloc tree deletion after merge_reloc_roots")
to a 5.4.x kernel before 5.4.54.
Unfortunately, the test can only run if several other patches that
fixed other bugs in d2311e698578 are applied or removed at the same time.
Commit d2311e698578 introduces a bug which destroys filesystems under test
long before tree mod log failures can be reproduced in testing. One of
those patches also fixes tree mod log issue #4. I do not know which one,
but since kernels after 5.1 cannot run without all of those patches, I do
not think it matters.
Tree mod issue #4 is the reason why the tree mod workaround is still
required on all kernels before 5.4. The issue still exists on older
LTS kernels, e.g. 4.9.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Rewrite the text related to 'btrfs send' to clarify that the send
workaround is no longer necessary to avoid kernel crashes, but still
useful because send and dedupe still do not work at the same time.
Replace "many backref code changes" with a specific commit reference,
and improve the grammar of some issue descriptions.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Apparently there's Github Flavored Markdown, and there's the markup
language that github uses, and they are distinct things.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Present known kernel bugs in table form with issue descriptions,
fixed and broken kernel versions, and references to fixes.
Update kernel version recommendations to include information on kernel
versions up to 5.8.14.
Reduce emphasis on data corruption bugs which are 1) two or more
years old now, and 2) much less bad than the bugs in kernel 5.1.
Add deprecation warning for kernels before 4.15.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Prefer to use cmark-gfm with extension 'table' so we can use tables in
locally-generated HTML files. If cmark-gfm is not installed then
fall back to some other Markdown implemeentation, but the tables will
be broken on every other implementation I have tried so far.
Also make the HTML output depend on the Makefile, since there may be
document translation options specified there (like '-e table' or an
entirely different Markdown implementation).
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
uncaught_exception() had only the one valid use case, and it can be
reimplemented by literally calling current_exception() instead.
current_exception() has several valid use cases, so it is not likely
to be deprecated any time soon.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We cannot use BeesContext::roots() until after
BeesContext::set_root_path() has been called.
Save up the parameter settings until then.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
"Storm of softlockups" starts with a simple BUG_ON, but after the
BUG_ON, all cores that are waiting on spinlocks get stuck.
The _first_ kernel call trace is required to identify the bug.
At least two such bugs have been identified.
Add some notes about the conflict between LOGICAL_INO and balance,
and the recently added bees workaround.
Update the gotchas page for balances to point to the kernel bugs page.
Remove "bees and the full balance will both work correctly" as that
statement is not true.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This avoids some kernel bugs. One of them is fixed in 5.3.4 and later:
efad8a853a "Btrfs: fix use-after-free when using the tree modification log"
There are apparently others in current kernels, so for now just put bees
on pause until the balance is done.
At some point we may want to provide an option to disable this
workaround; however, running bees and balance at the same time makes
neither particularly fast, so maybe we'll just leave it this way.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
In version 2.30 glibc added it's own gettid() function. This resulted in
"error: call of overloaded ‘gettid()’ is ambiguous" because gettid()
now exists in both namespace crucible and std.
For now, use explicit references to namespace crucible. This continues
to work with new and old libc without having to test specific library
versions.
At some point, glibc gettid() will be deployed widely enough that we can
remove the crucible version entirely.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Localize the hash function in bees to a single spot to make it easier
to change later (or at runtime).
Remove some code that was using a property of CRC as an optimization.
The optimization doesn't work for other hash functions, and running the
CRC function takes more CPU time than the optimization saved.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
CityHash64 appears to be the fastest available block hashing algorithm
that is good enough for dedupe. It takes much less CPU than the CRC64
function, and avoids hash-collision problems with file formats that use
CRC64 as an integrity check on 4K block boundaries.
Extracted from git://github.com/google/cityhash with the "CRC" hash
functions (which require Intel/AMD CPU support) removed. We don't
need those, and they introduce a new (if only theoretical) build-time
dependency.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
It is not possible to emulate extent-same by clone in a safe way.
EXTENT_SAME has been supported in btrfs since kernel 3.13, which
is much too old to contemplate running bees on.
Remove this dangerous and unused function.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We are getting a lot of exceptions when an inline extent is too large for the
TREE_SEARCH_V2 buffer. This disrupts ExtentWalker's extent boundary
search when there is an inline extent at the beginning of a file:
# fiemap foo
Log 0x0..0x1000 Phy 0x0..0x1000 Flags FIEMAP_EXTENT_NOT_ALIGNED|FIEMAP_EXTENT_DATA_INLINE
Log 0x1000..0x2000 Phy 0x7307f9000..0x7307fa000 Flags 0
Log 0x2000..0x3000 Phy 0x731078000..0x731079000 Flags 0
Log 0x3000..0x5000 Phy 0x73127d000..0x73127f000 Flags FIEMAP_EXTENT_ENCODED
Log 0x5000..0x6000 Phy 0x73137a000..0x73137b000 Flags 0
Log 0x6000..0x7000 Phy 0x731683000..0x731684000 Flags 0
Log 0x7000..0x8000 Phy 0x73224f000..0x732250000 Flags 0
Log 0x8000..0x9000 Phy 0x7323c9000..0x7323ca000 Flags 0
Log 0x9000..0xb000 Phy 0x732425000..0x732427000 Flags FIEMAP_EXTENT_ENCODED
Log 0xb000..0xc000 Phy 0x732598000..0x732599000 Flags 0
Log 0xc000..0xd000 Phy 0x7325d5000..0x7325d6000 Flags FIEMAP_EXTENT_LAST
# fiewalk foo
exception type std::system_error: BTRFS_IOC_TREE_SEARCH_V2: /tmp/foo at fs.cc:844: Value too large for defined data type
Normally crawlers simply skip over inline extents, but ExtentWalker will
seek backward from the first non-inline extent to confirm that it has
an accurate starting block for the target extent. This fails when it
encounters the first inline extent.
strace reveals that buffer size is too small for the first extent,
as seen here:
ioctl(3, BTRFS_IOC_TREE_SEARCH_V2, {key={tree_id=258, min_objectid=78897856, max_objectid=UINT64_MAX, min_offset=0, max_offset=UINT64_MAX, min_transid=0, max_transid=UINT64_MAX, min_type=BTRFS_EXTENT_DATA_KEY, max_type=BTRFS_EXTENT_DATA_KEY, nr_items=16}, buf_size=1360} => {buf_size=1418}) = -1 EOVERFLOW (Value too large for defined data type)
Fix this by increasing the buffer size until it can handle the largest
possible object on the largest possible btrfs metadata page (65536 bytes).
BtrfsExtentWalker already has optimizations to minimize the allocation
cost, so we don't need any changes there.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Some build environments (ARM? AARCH64?) do not have the fields
si_lower and si_upper in siginfo.
bees doesn't need them, so don't try to access them.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Update the version ranges on the dependencies.
FIXME/TODO: start dropping early versions that don't work with current
code?
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
flushoncommit or not-flushoncommit isn't really a bees matter--it's
a sysadmin's tradeoff between reliability and performance. bees does
not affect that tradeoff because all dedupe src extents are flushed, so
bees introduces no *new* data loss risks in the noflushoncommit
case--i.e. any data that you could lose while running bees, you'd also
lose when not running bees.
Note that the converse is not true: bees might trigger flushing on
data that would not normally have been flushed with noflushoncommit,
and improve data integrity after a crash as a side-effect of dedupe
operations. The risks of noflushoncommit might be reduced by running
bees. I don't have evidence based on experimental data to support that
conclusion, so I'll just leave this possibility as a rumor in a commit
log message.
lvmcache can be moved from the "bad" list to the "good" list now.
bcache remains in the "bad" list due to some non-data-losing failures
that only seem to happen with bcache.
Add a note about CPUs with strange endianness or page sizes, as nobody
seems to have tried those.
Remove "at great cost" from the btrfs send workaround. The cost is
the cost, there is no need to editorialize.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The existence of information about known data corruption bugs should be
visible from the top-level page.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
* comprehensive list of kernels with bees-triggered corruption bug fixes
* deadlock between dedupe and rename is now fixed (in some places)
* compressed data corruption is now fixed (in more places)
* btrfs send fix for one bug is now merged in 5.2-rc1, another bug remains
* retired the bcache/lvmcache bug (can't reproduce those bugs any more,
although I *can* reproduce an interesting non-destructive bcache bug)
* new minor bug entries for two harmless kernel warnings
* new entry for storm-of-soft-lockups
Fixes: https://github.com/Zygo/bees/issues/107
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This is especially useful when dynamic load management allocates more
worker threads than active tasks, so the extra threads are effectively
invisible.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This may help users understand some of the things that happen inside
bees...or it may just be horribly long and confusing.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Enable much simpler Task management: each time a Task needs to be done
at least once in the future, simply invoke the run() method on the Task.
The Task will ensure that it only runs once, only appears in a queue
once, and will run again if a run request is made while the Task is
already running.
Make the queue policy a member of the Task rather than a method. This
enables Tasks to reschedule themselves, possibly on the appropriate queue
if we have more than one of those some day.
This happens to make Tasks more similar to Linux kernel workers.
This similarity is coincidental, but not undesirable.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Introduce a mechanism to suppress exceptions which do not produce a
full stack trace for common known cases where a loop should be aborted.
Use this mechanism to suppress the infamous "FIXME" exception.
Reduce the log level to at most NOTICE, and in some cases DEBUG.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
libcrucible at one time in the distant past had to be a shared library
to force global C++ object initialization; however, this is no longer
required.
Make libcrucible static to solve various rpath and soname versioning
issues, especially when distros try (unwisely) to package the library
separately.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We stopped supporting shared hash tables a long time ago. Remove comments
describing the behavior of shared hash tables.
Add an event counter for pushing a hash to the front when it is already at
the front.
Audited the code for a bug related to bucket handling that impairs space
efficiency when the bucket size is greater than 1. Didn't find one.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Capture SIGINT and SIGTERM and shut down, preserving current completed
crawl and hash table state.
* Executing tasks are completed, queued tasks are paused.
* Crawl state is saved.
* The crawl master and crawl writeback threads are terminated.
* The task queue is flushed.
* Dirty hash table extents are flushed.
* Hash prefetch and writeback threads are terminated.
* Hash table is deallocated.
* FD caches and tmpfiles are destroyed.
* Assuming the above didn't crash or deadlock, bees exits.
The above order isn't the fastest, but it does roughly follow the
shared_ptr dependencies and avoids data races--especially those that
might lead to bees reporting an extent scanned when it was only queued
for future scanning that did not occur.
In case of a violation of expected shared_ptr dependency order,
exceptions in BeesContext child object accessor methods (i.e. roots(),
hash_table(), etc) prevent any further progress in threads that somehow
remain unexpectedly active.
Move some threads from main into BeesContext so they can be stopped
via BeesContext. The main thread now runs a loop waiting for signals.
A slow FD leak was discovered in TempFile handling. This has not been
fixed yet, but an implementation detail of the C++ runtime library makes
the leak so slow it may never be important enough to fix.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We need to replace nanosleeps with condition variables so that we
can implement BeesContext::stop. Export the time calculation from
sleep_for() into a new method called sleep_time().
If the thread executing RateLimiter::sleep_for() is interrupted, it will
no longer be able to restart, as the sleep_time() method is destructive.
This calls for further refactoring of sleep_time() into destructive
and non-destructive parts; however, there are currently no users of
sleep_for() which rely on being able to restart after being interrupted
by a signal.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Add a method to have TaskMaster discard any entries in its queue, terminate
all worker threads, and prevent any new Tasks from being queued.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The deadlock seems to be fixed now (if there ever was one--there certainly
were deadlocks, but matching deadlocks to root causes is non-trivial
and a number of distinct deadlock cases have been fixed in recent years).
The benchmark data is inconclusive about whether it is better to fsync or
not to fsync. A paranoia option might be useful here.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The crawl_master task had a simple atomic variable that was supposed
to prevent duplicate crawl_master tasks from ending up in the queue;
however, this had a race condition that could lead to m_task_running
being set with no crawl_master task running to clear it. This would in
turn prevent crawl_thread from scheduling any further crawl_master tasks,
and bees would eventually stop doing any more work.
A proper fix is to modify the Task class and its friends such that
Task::run() guarantees that 1) at most one instance of a Task is ever
scheduled or running at any time, and 2) if a Task is scheduled while
an instance of the Task is running, the scheduling is deferred until
after the current instance completes. This is part of a fairly large
planned change set, but it's not ready to push now.
So instead, unconditionally push a new crawl_master Task into the queue
on every poll, then silently and quickly exit if the queue is too full
or the supply of new extents is empty. Drop the scheduling-related
members of BeesRoots as they will not be needed when the proper fix lands.
Fixes: 4f0bc78a "crawl: don't block a Task waiting for new transids"
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
If /bin/sh is bash, the 'type' builtin produces a list of filenames
that match the arguments to $PATH.
If /bin/sh is dash, we get errors like:
/bin/sh: 1: P:: not found
Hopefully having a build-dep on bash is not controversial.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The two files are identical except README.md links to docs/* while
index.md links to *.
A sed script can do that transformation, so use sed to do it.
This does modify a file in git, but this is necessary to make all
the Github views work consistently.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This sequence of log messages isn't clear:
crawl_master: WORKAROUND: Avoiding RO subvol 6094
crawl_master: WORKAROUND: RO root 6094
The first is from a cache miss, and appears wherever a root is opened
(dedupe or crawl). The second is skipping an entire subvol scan, and
only happens in crawl_master.
Elaborate on the second message a little.
Also use the term "root" consistently when referring to subvol tree IDs.
btrfs refers to these objects by (at least) three distinct names: tree,
subvol, and root. Using three different words for the same thing is worse
than using a single wrong word consistently to refer to the same concept.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
After weeks of testing I copied part of a change to main without copying
the rest of the change, leading to an immediate segfault on startup.
So here is the rest of the change: limit the number of
BeesContexts per process to 1. This change was discussed at
https://github.com/Zygo/bees/issues/54#issuecomment-360332529 but there
are more reasons to do it now: the candidates to replace the current
hash table format are less forgiving of sharing hash tables, and it may
even become necessary to have more than one hash table per BeesContext
instance (e.g. to keep datasum and nodatasum data separate).
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
https://github.com/Zygo/bees/issues/91 describes problems encountered
when running bees on systems with many CPU cores.
Limit the computed number of threads (using --thread-factor or the
default) to a maximum of 8 (i.e. the number of logical cores in a modern
laptop). Users can override the limit by using --thread-count.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
options.md was a disorganized mess that markdown couldn't parse properly.
Break the options list down into sections by theme. Add the new
'--workaround-btrfs-send' option to the new 'Workarounds' section.
Clean up the rest of the text and fix some inconsistencies.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Introduce --workaround options which trade performance or effectiveness to
avoid triggering kernel bugs.
The first such option is --workaround-btrfs-send, which avoids making any
modification to read-only subvols to avoid btrfs send bugs.
Clean up usage message: no tabs for formatting, split options into
sections by theme.
Make scan mode a non-static data member like all (most?) other options.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The log message is quite CPU-intensive to generate, and some data sets
have enough hash collisions to throw off benchmarks.
Keep the event counter but drop the log message.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Make sure the result set is empty before running the ioctl in case
something tries to consume the result without checking the error status.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Zygo Blaxell