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.
This gets us to being able to run reads during a checkpoint, but
now we have to wait for new reads to end before we can release
the write lock, etc.
This is actually slightly slower, but if we could get ONE more step,
we could allow new writes during that phase, to a different WAL,
if we had a different WAL to write to.
PageMap uses "WALID", which is a WAL page ID relative to the "base" ID of the
WAL, rather than the wal page count you'd get just reading the file. So everything
it reports has a fixed offset at any given time. I think this may be left
over from a point where there were partial checkpoints. Anyway, the net
outcome is that each new transaction was getting different page IDs, but
the actual WAL pages did not always reflect that. Each checkpoint increases
the offset. This might imply that we can start having problems after
4 billion pages written even if most of them were redundant?
Anyway, with that fixed, this seems to work. I think.
We change nothing substantive here, except that there's a window
between when a write transaction updates the root pages and when
it removes itself from the db tx list and possibly causes a checkpoint
where it's not holding the db lock.
The issue here is that we want to be able to *keep* the lock but
still return, so no one else can start transactions, but the specific
Rollback or Commit that removed the last outstanding transaction
doesn't block forever. This will, later, allow us to exercise
finer-grained control over when we allow transactions. This is
a separate commit so we can run the test suite against it, and
verify that this part in particular didn't break anything.
Discovered test was running slightly strange and spending an unreasonable
amount of time on rand.Intn(), possibly because we weren't caching the
value used as the loop condition. Tweaked that, also made the pool a
bit different. Now it takes ~50 seconds for benchtime 100x, and produces
a profile with a TON of time spent waiting on sleeps (expected) and
the condition variable for waiting on checkpoints (the thing we want
to measure, really).
Note also the commented-out debug printf in checkpoint, there as a
reference. This is interesting because it turns out that MOST of checkpoint
writes is not actually writing new pages in most cases.
The actual "pages in WAL : pages in map" ratio is typically around 30:1
apparently. This would likely be different in cases where we were
updating existing data, though.
This is scratch space to prep for an actual work. The final
results will likely be different.
i used this script, a little clunky but it got the job done
```bash
for file in `find . -type f -print | grep '\.go'`; do
sed '1,/^\/\/ limitations under the License.$/d' $file > $file.tmp;
result=`cat $file.tmp`
if [[ result != "" ]]; then
gofmt $file.tmp &> /dev/null;
if [[ $? == 0 ]]; then
mv $file.tmp $file && gofmt -w $file;
else
rm $file.tmp;
fi
else
rm $file.tmp;
fi
done
```
