We centralize the creation paths for test indexes, fields,
etcetera so they all have a common path, all using standard
test holders. There's still two versions, one for test.* functions
and one for internal. They do share a TestHolderConfig though.
Large hunks of the related APIs are simplified/streamlined.
* Fragments are always created with a Field and don't need
a workaround in case they don't have it.
* Creation of test fragments, etc., use optional FieldOptions
but don't specify names because they're all using new holders
for each thing created anyway. This dramatically reduces
the complexity of the calls.
* test fragments are created inside test views which are created
inside test fields, etcetera, so everything is using the same
logic; test views aren't bypassing the other layers, they're
creating themselves normally within a field.
* Quite a few things now use the standard runtime/production
logic instead of being custom workarounds; for instance, instead
of `mustOpenMutexFragment` creating a fragment and then creating
a mutex vector for it, we just create a mutex-typed field and
have the normal runtime code do this.
* Similarly, we now use the same field creation logic that production
does, instead of having our own test-only thing that validates
field names directly, so our test that we're validating field names
is actually testing the runtime code. Yay.
* fragSpec goes away. it was a replacement for fragProxy which existed
to solve memory allocation problems but replaced them with interface
overhead problems. Now we just have pointers to things and maintain
valid data structures.
* Many panics are now Fatal or Fatalf calls.
* Some specific bugs fixed, like a cluster which was requested and
then had its first node directly overwritten, which isn't valid with
shared clusters.
* Drop the temp-dir test flag and TempDir variable, we can just use
$TMPDIR.
* Drop a benchmark of "write file to disk" that was purely a benchmark
of file write speed, not a benchmark of rendering the data that needs
to be written.
* Drop the unused "flags" parameter to fragment creation, which was
only used back when we changed the BSI format.
* Use holder.Txf() rather than index.Txf(). The TxFactory has to be
holder-level anyway, referring to it via the index is misleading.
* Test holders automatically close themselves and delete themselves,
we remove various other things that thought they were responsible
for deleting themselves.
A few view functions were taking a Tx, which had to be shard-specific,
but that's sort of awkward -- the view is inherently not shard-specific,
so it should be handling sharding internally.
There were also a couple of remaining obsolete checks for whether a
Tx was nil, at least two of which were in contexts where it absolutely
can't be. Remove all of them, and also the function itself.
The "field/view will just synthesize a tx" behavior is awful and
also hides a number of fundamental flaws. We distinguish between
"we really do mean to work on a single shard here" and "we intend
to work on the whole field or view", and the latter now take
Qcx instead of Tx.
This eliminates a lot of very weird cases where we checked for
nil Tx and synthesized them, and also gets us away from
field and view taking Tx parameters when no possible Tx
can be constructed which is valid, because Tx are inherently
shard-specific at this time.
break in a select in a for terminates the current case of the
select, but does not terminate the for loop. The worker queue
implementations for opening indexes/fields/views all suffered
from the same issue here.
Also fix a `<= 0` on a uint value.
All hail staticcheck.
When starting up, we can have a large number of views, each
with some number of fragments, and by default these were being
opened sequentially. There's no real benefit to that; they're
all nicely independent from each other and don't need much
locking, so we implement a trivial semaphore and launch the
operations asynchronously. We also combine them into
errgroups.
Similarly, we do this for fields and views, capping the number
of fields (or views) opened in parallel to avoid hitting a
system-wide limit on threads created (oops). Note that the
limits are shared, not multiplicative; we cap this fairly
arbitrarily at 8 fields being opened, and 16 views being opened,
at a time, but NumCPU*2 fragments being opened by those views.
This dramatically increases CPU load during startup, but doesn't
seem to significantly increase total CPU time, it just scales
much better on machines with lots of cores.
As the size of a fragment grows, the cost of snapshots
increases; with a large fragment getting a lot of large writes,
every write will trigger a snapshot, while any other writes have
to wait for that snapshot before they, too, can trigger a snapshot.
To address this, we introduce a background queue of snapshots.
In general, operations which were omitting their ops log writes
and just snapshotting no longer do; they emit an ops log. This does
mean that, in some cases, the ops log is written and then a snapshot
takes place essentially immediately, which costs us some performance.
However, that only actually happens under very light load; under
heavier load, there's generally going to be multiple writes coalesced
into each snapshot, and the ops log writes for them will be much
cheaper than a full snapshot.
When we do a snapshot, we may end up with containers which are
mmapped to the old file, and containers which have allocated storage
identical to the contents of the new file. It would be nicer if they
were mapped to it. But unmarshalling the entire file is expensive.
Instead, we remap it. (Or, if we couldn't mmap it, just make sure
the old stuff is no longer using the old storage space before we
munmap it.)
If you delete a fragment while something else is calling allFragments,
you can cause a race. This almost never happens in practice, because
deleting fragments is rare, and the only likely overlap would be with
something like the holder cache flush, which only happens once a
minute. But if you slowed down the rest of the tests enough, and ran
with -race, you might see it.
We check v.fragments directly instead of calling v.Fragment, because
v.Fragment also needs a lock, and we don't want to drop the lock between
the check for existence and the delete operation.
So it's true that Rename's arguments are called oldname/newname, and
you want to rename from the previous name to the new name.
And it's true that we're calling Rename on oldPath and newPath.
But in our case, oldPath is the name the fragment file had before
the operation, and newPath is the name of the temporary file
created during the operation. Use tmpPath and frag.path to make
the semantics clearer.
This commit implements BSI with variable bit depth using a
sign magnitudeto indicate whether a value is positive or negative.
This also rearranges the existence bit to be the first bit instead
of the last bit.
So with the switch to a new linter, we get a lot of new warnings,
and the majority of them are harmless probably, but a few might be
real. Variously just use _ to suppress warnings, or report errors.
There's probably things here that deserve better fixes, but we can
always revisit it.
There existed a case where two goroutines would try to
CreateIfNotExists the same fragment, and the first would
create it, but not put it in the fragments table, then
drop the lock, try to broadcast a message, and if it
succeeded then populate the fragments table. The second
would come along during the broadcast, not find an
entry, try to create one, and fail because the file was
already locked.
Basic problem: At least one test in server/ will fail
if we don't delay to send out broadcast messages. Everything
will lock up if we can wait forever (or even just a very
long time) for the message broadcast. We don't ever want
to have an inconsistent state -- so we don't want to either
fail to get a fragment when one's been created, or get one
that's about to be deleted if the broadcast fails.
So, creation and stashing in the fragments table is
atomic and immediate. After that, we optimistically attempt
to broadcast. If we fail, we fail. We delay up to about
50ms for the broadcast to be done, but after that return
anyway. This way, if things are going well everything
works, and if there's unexpected delays, things work except
some nodes in a cluster may not know about available
shards on other nodes sometimes. But that would have
happened anyway. A proper fix is beyond the scope of this
patch.
I'd like to add stat tracking to Roaring, which means it
has to be able to import the stats package, which means
stats has to be a package rather than part of the pilosa
package. If stats stops being in pilosa, it still needs
a way to import logger, so logger also has to leave the
pilosa package. Then everything using them needs to import
them and use package selectors on their names.
This doesn't actually add the stats support to roaring,
it just makes it so there's a way to import the stats
code from something in the roaring package.
This commit removes the previous `MaxShard` tracking and replaces
it with an `Available Shards` set tracking. This allows sparse shard
tracking without implicitly tracking all shards in between.