We switch everything to use QueryContext/QueryRead/etc instead
of Qcx/Tx.
We drop the short_txkey subpackage (it's now handled by either
keys or querycontext).
We drop all the dbshard stuff, and all the tx/txfactory stuff.
We remove all the things that related to the old "Block" concept,
which was mostly used by the anti-entropy code, but had one
fragmentary usage left in the ImportRoaringOverwrite case of
ImportRoaring. That's replaced by using a rewriter that deletes
all bits (not just bits in specific columns) from an existing
thing, but writes in new bits. Actually we could probably do that
better with a custom "eradicate-rewriter" that doesn't try to
be clever, and just eliminates things.
This includes a number of minor bug fixes that were
exposed by getting the testing to work. For example:
* When checking whether an operation "requires write", we
now consider a Delete a kind of a Write, because it is.
* Several tests were relying on the fact that writes through
Qcx were being committed whether or not the Qcx was ever
told to finish. With QueryContext, you actually have to
reach a Commit() or the writes don't happen (except for
special cases in Delete).
* Replaced a lot of panics with t.Fatalf in tests.
There's also some minor staticcheck fixes, like deleting the
unused "db" member of a boltdb transaction wrapper.
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.
When doing tests, we create a ton of one-off clusters. This
turns out to be expensive and slow. Fixing it is surprisingly hard.
Fundamentally: If we're sharing clusters, we need to use different
indexes for each test, to avoid clashes. This changes index names.
As a side-effect, this reorders many partition-based things, like
the order keys are returned in. Thus, to fix this, we change a lot
of tests to no longer depend on the *order* in which strings are
returned.
Having done that, we can also discard the ModHasher behavior, since
that only existed to allow us to reliably predict partitioning.
The basic design is as follows: Instead of a cluster being a
[]*Command, a "shareable" cluster is now a []*Command plus some
flags, and a "cluster" is a pointer to a possibly-shared cluster,
plus a link to the specific test using this specific cluster,
and correspondingly, its test name suitably coerced to be a valid
index name prefix.
The "test.Cluster" object now has methods to allow retrieving an
index name, and also implemnts fmt.Formatter to let you use,
e.g., `%i` with it in Sprintf to get "the index name, plus an i".
(This works for everything but %p and %T.)
This allows us to consistently rework all the many things that
use index names in a persistent way.
We also have `MustUnshared` and `MustRunUnsharedCluster` methods
which allow us to specify that a given test needs its own cluster
for some reason. For instance, the tests that want to run backups
need their own isolated cluster, and the tests that want to close
or reopen nodes need their own cluster because a reopened cluster
won't have working GRPC for some reason.
On "closing" a shared cluster (actually the test-specific wrapper
that reflects a given sharing), we delete any indexes starting with
that test's index name prefix. Otherwise, the huge pile of open
indexes prevents `go test -race` from working on MacOS, where we
run out of address space too quickly.
This is fairly enormous but most of the individual changes are
fairly trivial things like replacing the string "i" with "c.Idx()".
We also tweaked a test that failed for me a couple of times to
not depend on sort order.
* create getter for monitor state
* refactor monitor
* fix http middleware
* change warn to error if attmpt to cluster on plg
* sentry: special considerations if execution is part of test
- skip test if they build a cluster as this will error by design
- skip sending messages to sentry if testing
* use mainline etcd-io dependencies, not forks
this commit does lots of things around clustering with goal of increasing stability.
- upgrades from molecula/etcd to go.etcd.io/etcd@v3.5.4
- upgrades from seebs/bbolt to go.etcd.io/bbolt@v1.3.6
- update tests to use unix sockets for etcd cluster communication
- this is what etcd uses for a lot of internal testing, so if their devs think
it's a valid test, we can probably accept that
- cleanup etcd node-watcher shutdown process
Co-authored-by: tgruben <tgruben@gmail.com>
* moved random query to another repo
it had weird dependency issues with upgrading to mainline etcd bc of the vegeta dep
so we removed it bc no one really uses it anyway
we got this error message:
```
github.com/molecula/featurebase/v3/cmd/random-query imports
github.com/tsenart/vegeta/v12/lib tested by
github.com/tsenart/vegeta/v12/lib.test imports
github.com/streadway/quantile tested by
github.com/streadway/quantile.test imports
.: "." is relative, but relative import paths are not supported in module mode
```
* add cleanup to EtcdUnixSocket test util
Co-authored-by: reesporte <reesedporter@gmail.com>
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
```
We disable fsync more consistently in testing, including using
etcd's already-existing UnsafeNoFsync option to disable fsyncs
in the backing store boltdb used by etcd, to reduce runtime of
our tests on MacOS significantly.
Corresponding to this, we update etcd by one patch to pick
up a locally-invented patch which turns out to be nearly-identical
to the upstream fix for "disabling fsync makes boltdb not
even bother to write some data sometimes", which caused crashes
galore.
A nil Qcx is a crime against existence and makes baby pandas cry.
Having taken out the hack that tried to accommodate this when tests did it,
we now have to fix the tests. Oh no.
A while back we started just polling the reported cluster state of one node
when starting a cluster for tests. This works fine if we're doing fresh
new etcd queries for every single operation -- but that's insanely
expensive, it turns out.
When we use the watcher, some nodes will report stale data for "a
while", where "a while" appears to be easily a couple dozen milliseconds.
This is probably irrelevant in most real-world cases, because the common
case (detecting a node going down) means that we have at least five
seconds after a node goes down before etcd notices the lease expiring,
and a few milliseconds more or less won't matter.
But we have tests that assume either that node 0 is always the
coordinator (wrong) or that waiting for node 0 to think the cluster
is up means that every node in the cluster thinks the cluster is up,
or at least that it means that the coordinator thinks the cluster is
up. We retried later operations but not the initial ones against
the coordinator.
In fact, we probably want to wait for the entire cluster to think
it's up before we start trying things on clusters.
We also replace the "CheckClusterState" function with the existing
AwaitState call, or a new AssertState which errors out since that's
the way we usually use AwaitState anyway.
In the AwaitPrimaryState function, which used to be
AwaitCoordinatorState in a different long-lost revision, we have
to delay until a primary node is available, or fail if one does
not become available, to avoid a panic. This probably shouldn't
happen anymore, because of the last change:
Also, rovide dummy topology.Node entries before metadata is read.
During initial startup, we want to be able to do things like determine
which node is the primary, even before we've read metadata from them.
To do this, we populate the node list with dummy entries that just have
the ID (the only part we need to sort our list), and a node state of
UNKNOWN.
This breaks the fancy logic for determining whether or not to update
the node data, because the initial status of UNKNOWN matches what we
get from SetMetadata giving us new data so we end up not realizing
that this was actually a meaningful change. But actually, that's
a pretty niche optimization; we usually only get state changes when
there's an actual change in state. The updates here are cheap
and only happen after a write (or on the first query) so it's not
worth making the logic a lot fancier to make it work, when we can
just do the simple thing and update any time the dirty flag is set.
We also standardize on a 50ms delay, because 1ms delays were
really expensive when each check was hitting etcd multiple times,
and 50ms is Usually Long Enough.
etcd runs a LOT more goroutines during server startup. Fix a
goroutine/for loop bug causing us to run four 7-node clusters
instead of 1/3/4/7-node clusters, also have the test/cluster
code reduce import workers. We can't do much about the spamminess
of the Raft stuff, but this should tone it down some.
Long story short: Once we create a server and start it, we can't start
it again. We can't close it and restart it, and we can't just start
it without closing it.
Unfortunately, if the server's config needs to change, we have a Problem
here.
This ultimately means that the retry logic for GetListeners can't actually
retry successfully; if we fail on the first attempt, we necessarily fail
on any later attempts also, and if we try to fix that, we get panics.
But!
We don't actually NEED to retry. We just need to ensure that we can
open a :0 port, extract the actual port number, and use that in places
where the port number mattered, without having to rebind it.
The only actual place we needed to rebind things was opening gRPC
servers, so we introduce a gRPC Listener that can be used instead of
trying to bind to a specified port.
In a bunch of other cases where we had similar logic to try to allocate
and then use a port, we can switch to just using a provided listener.
For instance, net/http has `Serve(net.Listener, handler)`, not just
ListenAndServe(addr, handler).
This should eliminate the weird CI failures from eaddrinuse.
NOT fixed: server/cluster_test.go/TestClusterResize_AddNode isn't working
right now. The new node isn't actually being added to the existing cluster.
I attempted this but was outsmarted by it, and I think fixing the
rest of this is worth it as a separate thing.
