We previously had some significant bugs in which our error handling for translation would cause an entire batch of queries to silently fail.
This change processes each call seperately, such that all executable queries are processed.
This was previously done for the path where an index did not have keys
and this commit does it for the paths which do have keys.
This was originally two commits as there was some tricky cases that
needed multiple else statments within a single type.
Had to remove QuerySQLUnary stuff from grpc_test.go since that was testing functionality which has added by the VDSM collapse and we're backporting this fix onto 2.1 which is pre-VDSM collapse
This can cause incredibly weird and hard-to-debug problems if the previous
container value is still in the cache after an update, and in particular,
can result in having a stale container value cached after a roaring import
that modified the container. Coupled with another bug which could corrupt
containers on a delete, this produces a very strange bug where a value is
present in a fragment, but an attempt to delete it reports failure.
Some cluster tests failed sporadically. In order to fix them, I
introduced some debugging-related functionality, which revealed
several new bugs that were actually existing bugs we just happened
not to hit in testing. This combines various fixes.
We start with "make the nodes used in testing have distinct names
based on the test case name", which lets us discover that we are
leaking clusters, which continue to sit around talking with each
other. That in turn causes significantly higher load on access to
ephemeral ports, which causes sporadic failures when we shut a
node down and try to restart it, but something else has gotten assigned
its ephemeral port number since then.
Part of the fix is to try to rebind on port 0 if an attempt to
bind to a specified port over 32k fails. This is a guess; the
actual ephemeral port range could be 16k+, 32k+, or 48k+, or just
about anything else really, but it seems reasonable in
practice.
There were bugs in the oft-repeated loops to await the cluster
achieving a given state, and it could hang forever if it didn't,
so we add a timeout and a standard function on the test.Cluster
type to handle that. Note that the timeout seems irrelevant; in
every case I've tried, a timeout of 0 is fine because the node
start doesn't complete until the cluster state has changed.
Add a method to test.Command to run a query, expecting a specific
result. Also clean up some of the formatting and generation of
queries, and allow parameterized (badly) queries. This lets us fix
a subtle bug, which is that test cases were depending on assumptions
about shardwidths. Also improve the diagnostic output from some of
these functions so test failures are more comprehensible.
But actually that dependency on shardwidths was ALSO revealing a
genuine underlying bug, which is that a node resize did not correctly
propagate the schema to a new node if there was no data present
on shards that node would own. We now also have a test case that
hits that (or would, if we hadn't fixed it).
Add comments explaining the server options parameters for MustNewCluster
and MustRunCluster.
Also, we implement the ReadFrom and WriteTo behaviors for
InMemTranslateStore, without which some of the cluster resize tests
fail. Props to the comment for specifically stating that they wouldn't
work if that happened, which probably saved me several hours of
debugging. The implementations may not be robust, but
InMemTranslateStore is intended to be used only in lightweight
and transient testing.
Check also on bit setting for cache/storage mismatches.
Also, add diagnostic printing to a couple of points inside roaring
where we think the code can do something wrong. All of these are
cases where what the code does is actually wrong -- we're leaving
it wrong because we want to confirm/deny that this is happening
when the strange behavior happens.
We also filter these bugs a little bit -- we only print some of
them when they would result in a change to a mapped-or-frozen
container, which would be a bug that could affect things. There's
actually cases where this is wrong -- if these bugs affected
something like one of the keep/filter rows in unsignedLT, that could
affect things -- but it shouldn't apply in the cases we're
concerned with, and without the filtering, our regular tests
produce about 500,000 of one of these messages, all from removing
the last bit in array containers in a test where they actually
won't be used again.
We also mark all the test containers frozen so they *will* show
errors if this happens to them.
