Quick checks aren't super helpful, but they can test random stuff
a bit, so let's keep them, but make them smaller. Also, let's cram
the random selections into the first quarter of a shard so we see
more container updates to fewer containers because that's probably
more interesting.
don't add/remove two million bits one at a time. instead, insert
small array containers, make them larger, replace them with bitmaps,
and so on. In short, we still create containers and change their
sizes in interesting ways, but we do it thousands of bits at a time.
This reduces runtime with -race from around 10-20 minutes to a second
or so.
We drop the "PartialContiguous" test because it was actually just
identical to PartialNoncontiguous, so far as I can tell, and not
really interesting.
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.
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.
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.
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
```
First, it is possible for us to end up allocating *or freeing* pages during
a modification of the free list, in a way such that the change to the free list
means that when we finish the modification which caused the allocate or free,
we've overwritten the inner change.
Second, when deallocating trees, we don't actually deallocate the branch nodes
themselves.
The former causes potentially severe data corruption. The latter causes us
to gradually leak pages in a way that we don't notice because we only run those
tests during the RBF tests.
The fix for this is surprisingly intricate, because of the counterintuitive
fact that *allocating* a page means *removing* things from the free list
(and thus potentially deallocating free list pages), while *freeing* a page
means *adding* things to the free list (and thus potentially needing to
allocate pages for the free list).
While modifying the free list, any allocations we need always just come from
the end of the file; we don't try to reuse free pages. If a page becomes
*deallocated* by a free list modification, we don't annotate it in the free
list at the instant that it happens; we stash that information until the
current modification of the free list happens, then iterate through any
such pages.
I am pretty sure there's virtually never more than one, and I don't actually
know that I can create a case wherein we'd end up with the nested case
firing, wherein removing a page from the free list causes us to remove another
page, but I think if the free list got large and cluttered and needed
rebalancing or something it could maybe happen.
These commits are hard to disentagle, and doing them separately means
re-modifying the same chunks of code several times before removing it,
and similar things.
Basically:
(1) Drop the bolt backend storage.
(2) Drop the blue-green wrapper that compares two backends.
(3) Drop unused or barely-used Tx API components from all the
remaining backends.
(4) Minor related cleanup to simplify things related to these.
The boltdb backend existed only to verify RBF. The blue-green wrapper
was mostly used to verify RBF, but in practice we had to do a lot
of working around that, and it introduced a lot of special cases.
Types removed:
IteratorFinder: Used only to implement the roaring iterator
on top of boltdb, and to complicate the way it worked in roaring.
Reverted the complications. Also unexport NewSliceContainers
which is used only for that outside of roaring's internals.
PortMapper from cluster_internal_test.go: Used only for a test
we removed early this year. Never used for anything else.
RawRoaringData: Totally unused.
TxStore: Totally unused.
Functions removed from Tx API, and sometimes corresponding
members were removed from structs:
* Dump: debugging code, I don't think I found any actually reachable
paths to it.
* Group: only used for debugging TxGroup stuff
* IncrementOpN: only used by fragment, fragment can increment its
own opN.
* Options: unused?
* Pointer: debugging only
* Readonly: used only to decide how to handle Tx in a TxGrp,
but we never add a non-readonly Tx to a TxGrp. Removed also all
the corresponding write-aware stuff.
* RoaringBitmapReader: Used exactly once, can just be a bm.WriteTo.
* Sn (and OpenSnList): Unused
* UnionInPlace: unused and conceptually-invalid; it didn't write
to storage and shouldn't have, and was just "create a bitmap
then call union-in-place", which we can do directly.
* UseRowCache: just checked storage.UseRowCache.
Other things removed:
The SetRequiredForAtomicWriteTx and ClearRequiredForAtomicWriteTx
functions go away, since nothing now seems to be using them? Same
for holder_internal_test's `testHasBit` and `testMustNotHaveBit`,
which were unused.
The DBPerShard "DeleteDBPath" and "HasData" functions and related
parts were mostly unused; took out the parts that were never
actually being reached.
Changed the API of one function to simplify special cases and
remove things:
* ImportRoaringBits had a special "data" argument which gave it
subtly different semantics for RBF and roaring (for roaring, it
could produce a roaring bitmap *with ops log*), didn't seem to
be adding much. Removed corresponding "readStorageFromArchive"
which is not otherwise used.
Also took out various debugging/dumping functions that were unused
and may have bitrotted.
Dropped a test from txfactory_internal_test, and the "pjobs"
code, because those two were the only things that needed Barrier
and thus idem, which lets us drop two more dependencies. We already
have errgroup for grouping things which want to terminate as
soon as one of them errors, approximately. To do better we'd have
to have context-threading, really.
Unbroke the WriteFragment test for non-roaring tests and made it
not roaring-only.
This commit fixes an issue where the root record cache is only
built when a write transaction successfully commits. However, if
no write transactions are occurring then the the cache is never
built and saved so it is recomputed on every read tx.
Due to lack of synchronization, this test would sometimes close the DB before terminating a transaction:
=== RUN TestTx_CommitRollback/SingleWriter
tx_test.go:132: db still has 1 active transactions; must closed before closing db
The test now waits for the goroutines to terminate before closing the DB.
- use short_txkey for rbf
- short_txkey breaks a bunch of bolt_test.go, so leave it on (long) txkey for now.
- remove SliceOfShards method from Tx interface
- the sync.Pool default uses little memory under CI.
- arena approach provides ability to control the maximum memory
used by rbf Cursors.
- cursor caching is adjustable with --rbf-cursor-cache
currently 0 by default (meaning use sync.Pool), and
larger than 0 meaning use an arena of this size.
With the arena, 20 or less is needed to pass CI.
- rbf test suite runs ~ 4x faster
- kitchen sink ingest test runs 16% faster.
- report TotalAlloc in CALLSTATs
fixes#1105
- blue_green for doing migration. Called before Holder.Open finishes.
- holdbkg.go added for index lookup. Less wedging between a deadlock and a race.
- fix fault under read-only map under lmdb at
TestExecutor_Execute_Row_Range/RowIDColumnID by doing cow in roaring.
- roaring -tags gofuzz builds again
- roaringparanoia build tag added to make test targets in Makefile
- add rbf.NewDBWithAllocZero for out-of-bounds memory checks
- .circleci/config.yml test-shardwidth-22 with large run container, kept OOM-ing we suspect.
Fixes#819
The testhook/ package provides an easy way to set up multiple
hooks to run before/after tests are run.
The audit hooks track open and closes of storage backends,
files, indexes, and holders, for example. A tempdir wrapper
creates temporary directories which are automatically cleaned up
when the test ends. Any kind of resource creation that
should be closed at test conclusion can be tracked. We
will complain at the end of the TestMain if resources are
leaking.
Leaks under go1.13:
We use a wrapper function which is a no-op for go 1.13, but actually
calls testing.TB.Cleanup in go1.14, so we can still build with 1.13 even though
tests will leak files all over the place there. Because of this,
don't run the testhook tests when using 1.13, as they'll always fail.
- the test/pilosa.go http client now times out after 10 seconds
to help diagnose hung server situations.
- Makefile targets added to get better progress reports.
- lmdb as a backend (lmdb.go)
(lmdb is the fastest known transactional storage backend)
- per Tx call statics report enabled with PILOSA_CALLSTAT=true (stattx.go)
- framework for per-shard db (dbshard.go)
- txfactory handles any pair under blue-green testing (txfactory.go)
- enable CGO in Dockerfiles for lmdb
- rbf had races around the new rootRecords cache in tx
- rbf tx needed a write lock on the db now that rootRecords are written
- added a global registry for rbfDB to correctly dedup instances
- implement DeleteFragment, DeleteIndex for rbf
- use badger style keys for rbf to allow content checksumming to be list
containers in the same order
- lots of other integration of rbf into pilosa layer.
