This adds a shard-based import endpoint which takes bitmap data for
all field types and imports data for the whole shard transactionally.
It uses the BitmapRewriter interface to try to intelligently allow for
setting and clearing bits simultaneously without multiple writes which
is especially helpful when ingesting into int-like fields, but also
allows clear-and-then-set behavior for set fields.
We've been seeing weird retention of Tx that shouldn't still be open, and
one possible explanation is that, until a Tx actually uses the freelist
cursor (either to allocate a page or to release it back to the freelist),
the freelistCursor statically stored in the Db object continues to have a
pointer to the previous Tx which used it, which allows a Tx, and thus its
dirty page map, to be retained forever.
I previously thought this should also nil out the page maps in the Tx, but
the more I think about it, the less I think that's a good idea. The actual
lifespan of a committed Tx should be quite short. If it *does* stick around,
it's beneficial to us as debuggers to see those large maps of dirty pages
sticking around. So after thinking about it a lot I decided not to do
that.
Similarly, when closing out a container filter (whether a filter or
a rewriter), zero out the Cursor, Tx, and filter and rewriter functions.
(We don't have to worry about the cursor's Tx, because the cursor gets
closed, which zeros its Tx and returns the cursor to the cursor pool,
too.) This likely matters a lot less, as the filters in the pool
get garbage collected "soon", but it still reduces the amount of
stuff being retained.
The filter and rewrite logic are unlocking and relocking but I don't
think they should. I think those locks were added early on during
testing of the filter stuff, but I don't think they should be needed,
and I've been unable to find a case where they were. I think probably
I had something where a ConsiderData function was trying to run a Tx.
This in a parallel to ApplyFilter/BitmapFilter which allows writebacks
while it's running. It's a write operation, so it needs a write lock
on the Tx, and needs to create bitmaps if they don't already exist.
The semantics are a bit messy and need better documentation still.
MacOS's firewall complains about a previously unknown app trying to
listen for network connections whenever we run go test. That's because
we *are* listening for network connections on arbitrary interfaces, not
just on localhost as we probably intended. Fix that.
We have code to correctly fill in cell.BitN when a leaf cell already
exists but isn't of the correct sort, but not to handle the case where
it already exists and *is* a BitmapPtr, but doesn't necessarily have
the right BitN value.
We create a lot of these during a large GroupBy query or anything else
that creates a ton of filters. Use a pool so we can reuse them, since
most of their data doesn't need to be zeroed out, and typical use
patterns have a lot of sequential creation of these short-lived things
within a goroutine.
We used to manually do this because we had a number of cases where
BitN wasn't being updated, but so far as we know we've fixed them
and we have run a fair amount of stuff with sanity checks on and
not hit anything, so eliminating the constant recounting on bitwise
containers seems like a win.
We have some tests that cover stuff like the DumpDot functionality,
but we don't need them to actually write to stdout during ordinary
testing. Dump to buffers which we politely ignore. Yes, we could have
used a dummy writer, but this way it's super easy to display the
contents if we find ourselves suddenly caring.
When closing, we need to wait for existing Tx to exit before truncating
files and unmapping things. This shouldn't matter, because we don't actually
close the DB until all transactions are done, normally... except for the
background usage-gathering task. But really, it's probably just better to
be conservative.
The actual logic is fancier than it looks. We can't hold db.mu.Lock during
this, or the existing Tx can't exit. So we first grab the lock, set the closed
flag, set up a waiter for all current Tx to exit, and then release the lock.
Now we wait on the current Tx exiting. Once that's done, we grab the locks.
Anything coming in that tries to start a Tx will fail out fairly quickly
because the opened flag is now false, so even if other things get those
locks before we do, they won't keep them or create new Tx.
This makes one test deadlock because it opens a Tx and never closes it,
so we change that test to close its Tx.
There's no correct timeout value here, really, but the intent
of this is that we first want to be sure that a second tx doesn't
successfully start before the first exits, and then that the second
*does* successfully start *after* the first exits.
Unfortunately, there's no guarantees on timely processing, and in
reality, CI can break us by waiting more than 10ms before we get
enough CPU time to do something. More generally, there's no way to
make a test like this work correctly -- no matter how long you wait
for the second Tx to start before closing the first one, it's always
possible that it *would* have started just a millisecond later even
without you closing the first one. And similarly, no matter how long
you give it to start when it's *supposed* to, it could always take
longer.
We could in principle just set this to wait for the second Tx to start
and rely on the test timeout killing us if it doesn't, but then we
don't get a useful message.
Let's optimistically hope that 10 seconds is long enough for a trivial
rollback to happen, since that doesn't need to imply writes. And I
think 50ms is a better bet for the first test, although that does
make this test close to 5x slower on non-CI hardware.
This affects TestTx_Remove, TestTx_DeallocateToFreeList, and
TestTx_RecreateBitmap, all of which were adding hundreds of thousands
of individual bits, or more, and all of which work just as well and
produce the same behavior using largeish containers.
This reduces race-detector-test runtime from about 20 minutes to
a couple.
The MultiTx test runs for a fairly long time but doesn't add much
value running that much longer, and there's no reason it should take
more than half the time we spend on this entire directory.
The Cursor datatype is quite large, and allocating them constantly for
ops is extremely expensive. To avoid this, we create a single stable cursor
that lives in the DB, and can be used for freelist modifications. Since the
freelist is only ever modified once at a time, this should be safe. We also
don't fully zero it between operations, we just reset the relevant parts.
Several changes. One is, we don't provide a `New` for pagePool, which
allows allocPage to check whether a page was returned, and thus, zero
pages which were found in the pool, or make new pages, but never zero
pages it just created with make. We then also make many more things
which were making pages use the pool.
Reuse the same page allocation for multiple header pages dumped into
the WAL; the bitmap header pages aren't stashed in our page map,
they're only written to the disk, so we don't need to make a new page
each time, we can just make one new page for the whole batch.
Internally in the pool, we pool pointers to [PageSize]byte, rather
than slices. sync.Pool needs pointer-like things. To store a pointer
to a slice, you have to heap-allocate the slice, also. So, instead
of heap-allocating copies of these slices, we just use pointers to
the raw data.
This commit changes the max WAL size calculation to double the
number of bitmap pages in the WAL as they require an extra header
page. Previously, this was causing the WAL to be overrun and
references to those pages were outside the mmap range and caused a
panic.
This commit fixes a bug in RBF where deleting all the elements in
a bitmap that has a depth greater than 2 will cause the root bitmap
to be a branch page with a cell count of zero. This breaks an
assertion in `readBranchCell()` which causes a panic post-commit.
A new assertion has been added to prevent a branch page from being
written with a zero count in the future.
We don't need a condition variable for a thing with a single waiter
which waits only once, and a data structure which only one side ever
modifies. That's a closable channel.
Two issues: First, there was a race condition because we were never
using the mutex for anything but the condvar broadcast, second, there
was no reason for the afterCurrentTx to need to maintain the list since
we already know where in the list we are when we are waking it up.
afterCurrentTx still wants to run with the db lock held, because
the degenerate case (no outstanding Tx) means that it will be running
with it held already. That's for another commit.
We need to update db.PageMap after we write the db, but before
we truncate the WAL, so new transactions don't pick up the old
PageMap and then get a truncated WAL.
Also, checkpoint should not abort if there's txs -- that's okay now.
