it was somewhat difficult to avoid ripping this out without also
touching some of the stuff that supports roaring backend. That's going
soon too, so no worries :)
There's a number of deeper issues here (the fragment is conjuring
up a Tx, for instance) but this helps.
Also use field.view() to get the view rather than accessing viewMap
directly without a lock. Also change field.cacheBitDepth to ratchet
upwards -- if we have multiple shards and some shards have lower
depths than others, we should use the highest as the cached value,
not the most recent.
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
```
SanityCheckMapping is specific to roaring bitmaps stored in-memory, if
we have an RBF backend, we shouldn't even try it, it'll just panic.
This implies that, in whatever circumstance we were hitting this, we
were getting an error back from the backend. We still need to address
that error, but to do that we need to know what it was, which we don't
if we panic.
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.
If the inner function that handles the open of storage and cache
fails, we close the fragment. If we closeStorage() before that,
then we can try to close the storage again, which causes a panic
when we try to mark the generation as Done again.
I was going to set f.gen = nil after marking it done, but I'm
not feeling safe about that -- there's too many places where
we check things about f.gen, and it seems unsafe. The generation
code should be removed at some point, because it all exists
as a workaround for not having any way to detect when reads are
"done", because we didn't want to do something huge and intrusive,
like adding the Tx system and requiring transactions to get
closed.
We support query parameters for details (default false) which
request additional data, and for a limit (default 0/MaxInt32)
on number of results returned to limit the amount of spam
produced if there's a lot of results. The simpler default
output should reduce load and runtime significantly, and the
ability to specify limits makes it easier to get reasonably
small responses.
There's some context support here, but the underlying filters
don't take contexts or check for them, which is probably
a flaw but might be a bit large to correct for this.
Despite being large, this set of changes is actually
fairly well contained within the mutex-checking code.
This implements a fairly straightforward sanity-check for mutexes,
implemented as a bitmapfilter at the fragment level, and with higher
levels combining results. There's two endpoints, an internal endpoint
which only checks the local node's shards, and an external one which
forwards requests (using the internal endpoint) to all the other nodes.
The internal endpoint does not do key translation, the external one
does.
The transmission format is a probably-inefficient JSON blob, and
returns data separated per-shard so we don't have as much merging
work to do.
This introduces a horrifying monstrosity function which tries to
sneakily corrupt mutex fields and which has to be exported (EWWWWW)
but which is only present in _test code (!??!! THIS WORKS WHY).
Also one typo fix in unrelated code caused by not wanting to keep
fighting with gofmt about this.
This is a design to let us write test cases for ingest with schema setup
and data in the json formats we want to use, and results as alternating
queries and expected results, so we can just create new test files and
run the tests against them. We also have to report back what we created
when creating things.
In the process of developing this, I noticed that the documentation describes
ingest schema as allowing more than one schema operation, but we didn't support
this, and also it wouldn't do much good because there was no way to do partial
things like "just add a field". Fixed.
Also we implement comparison for ops, so the test output is actually
a test rather than just some data to visually eyeball.
In the process, realize that the handling of timestamps was wrong; we said that we
take them as raw numbers relative to the epoch, not as raw Unix timestamps.
Also a couple of related cleanups caught by doing the testing.
This is a rework of Nia's radix sort. Still using stdlib sort for the
tail ends of things, and should probably replace it at some point
because it's still woefully inefficient, but this gets decent
performance, and lets us do the fancy thing of doing quick partial
sorting by record-key-only to get to shards, then deciding whether
to sort by value-then-record (as for a set field) or just by record
(as for int fields), which lets us reduce the amount of re-sorting
the same data by different criteria we do.
We also use a messy code-duplication basically-bubblesort for the
inner loops because it's much cheaper for small N.
This also lets us use field-aware sorting for shards, sorting them
correctly for a corresponding field type, and add corresponding API
support and fragment support for an option to tell the fragment
code that we already ordered things in the order that's most
efficient there, to avoid a second sort that we don't otherwise
need.
This partially-implemented prototype of the ingest API is based on our
programmatic ingest API reference. It has noticable limitations, most
crucially that it doesn't handle multi-node clusters right now. However,
it basically implements the expected semantics.
There's some noticeable performance issues to do with the high overhead
of sorting bits in order to import them efficiently, but this is fixable.
We also add the hooks to the internal client, and make the finisher logic
a bit smarter.
Much of this code was originally by Nia Weiss, but it's been merged
and restructured a bit to get things broken into logical commits.
This works around an issue where unreplicated keys will not be matched everywhere.
This also avoids the cost of creating millions of bolt read transactions and allocating strings.
Since we don't always have "snapshots" anymore, the arguable benefit of
avoiding the snapshot is reduced, and the primary expense of
importPositions has been dramatically reduced as well, so let's
just use that all the time, and simplify life.
We also want to make it faster. We don't know how many bits there
are to set or clear in the input set, but we do know exactly how
many bits there are to set AND clear. We can subdivide these into
batches by rows, then process each batch by storing sets at the
bottom and clears at the top. We can also do batches by columns,
reducing the memory overhead of unpacking all the bits at once.
(For extra credit, we could alternate set/clear settings, and
thus do batches of "the clears from row 0, followed by the clears
from row 1" and "the sets from row 1, followed by the sets from
row 2", and so on, but this is too fancy.)
Every caller of importValue is in fact already providing values
with column IDs sorted. As such, we don't need a map for checking
the previously-set columns; we just need to check against the
previous value.
Attributes are unmaintained and unused.
They have become more of a liability than a benefit.
This change eliminates them from the codebase.
The only user-visible change (assuming that attrs are not used) is that the attrs field will no longer appear in row JSON.
When writing things that cause additions to the cache, mark it dirty and
flag it for recomputing, but only sometimes actually do the recalculation,
currently implying a 10-second window. We still mark the cache dirty,
so if a request comes in, we'll get fresh data, but the query will be
slowed down because the recomputation will happen then. But that's better
than doing thousands of recalculations which are never used...
The "batched" flag creates a complexity which is that the return value of Add
might or might not be meaningful, but it doesn't really buy us very much.
If we are concerned about the ops log size of writing single ops as 21-byte
arrays of 1 op rather than as 13-byte ops, we can make the AddN code smarter
about how it writes ops. And probably should.
Along with this, change Remove to use the batched operation form, which
writes a more meaningful ops log, and return a meaningful value for changes
made. Otherwise, it ends up writing potentially thousands of ops to the
ops log without reporting any OpN, because the number of ops written isn't
the same as the number of changes those ops made. This could result in
files growing by megabytes without OpN changing.
There was a comment here about a test failing with RemoveN. I can't prove
it, but I strongly suspect that this was actually a result of that test
case hitting a particular bug that we eventually fixed, and which we might
have fixed sooner if we'd realized why using RemoveN made that test
fail.
When *fragment.openStorage is invoked in both f.importValue and
f.importValueSmallWrite and it returns an error, this means there's
some underlying error with the storage device and at the point of this
commit, the sane thing to do is to close the process, otherwise the
operation of Pilosa might proceed in an inconsistent state thus
precipiatting other silent but hairy errors along the way such as
dereferencing *fragment.gen later on which is set to nil once
openStorage fails.
