This migrates existing code from the old TranslateKey(s) endpoints to the newer CreateKeys and FindKeys endpoints.
The CreateKeys and FindKeys endpoints were created previously as the TranslateKeys endpoint had no way to behave sanely when the looked-up key did not exist (the parallel-arrays representation did not have a good way to represent a missing key).
This change also removes the old TranslateKey(s) functions from the translation stores.
It leaves a wrapper emulating the TranslateKey(s) endpoints so that old idk still works for now.
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.
There's no need to have two different translation readers, a single
reader can handle both partitions and fields at the same time, so we
can combine them. This may not actually change things much but was
a useful step in diagnosing a different problem with translate readers,
and I think it is a minor improvement so I'm preserving the patch
just in case.
- the -fix flag repairs replication errors by copying from the primary.
- the -fixkeys flag repairs any string key translation issues.
- make pilosa-fsck installs pilosa-fsck and builds release-pilosa-fsck.COMMIT.GOOS.tar.gz release tarbar
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.
This commit adds `TranslationSources` to the cluster
`ResizeInstruction`. These are the sources of translation
partitions which the receiving node needs in order to support
partition distribution in the new, resized cluster.
This also fixes a bug where index options were not being
encode in the proto Index object. That meant that the schema
transferred via protobuf was not correct. The reason why
things normally worked is because index creation typically
happens on the CreateIndex message, which does include the
options.
TODO:
- [ ] implement the TranslateStore interface for `InMemTranslateStore`
and `mock.TranslateStore`
- [ ] surely need some more tests around the `ReadFrom` and `WriteTo`
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.
This commit fixes an issue where translation `LogEntry` must be
read in its entirety, however, large entries can exceed the buffer
size. This has been changed so that partial entries reads are allowed.
The `LogEntry.ReadFrom()` may still generate large byte slices
during reads of large individual fields or keys.
Depending on where in the replicate() loop you are when a
store is closed or reassigned, it's possible for it to deadlock.
The deadlock would be that replicate has just successfully read an
entry from your PrimaryTranslateStore.Reader, when a new
PrimaryTranslateStore event happens. Then handlePrimaryTranslateStore
grabs the mutex, signals that the replication handler should
close, and waits for the replication handler to close. Meanwhile,
the replicate() loop tries to grab the mutex... and deadlocks.
Solution: Make the replicate() loop part that needs the mutex
a goroutine that signals on a channel, so we can put it in a select
along with checking for the replicationClosing signal (or the
context terminating). If one of those happens, replicate()
terminates, allowing monitorReplication() to return, which
causes the anonymous function which called it to call
repWG.Done(), allowing handlePrimaryTranslateStore to continue
and eventually release the mutex. At some later point, appendEntry
succeeds or fails, dumps its result status in a buffered
channel, and exits, and the buffered channel is garbage collected.
This is way simpler than it sounds, but it took me a while
to figure out how simple it was.
In the case where a translate log entry contained
many key/id pairs, it was possible for the read
buffer (which was allocated at 65536 bytes) to
fail to handle it. This happened when the serialized
LogEntry was larger than 65536 bytes.
This PR adds logic which returns a custom error called
ErrTranslateReadTargetUndersized notifying the reader
to reallocate a larger read buffer and try the read
again.
TODO:
- [ ] Add a max buffer size check to prevent this from doubling the
buffer size with no limit.
- [ ] Add tests.
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.
monitorReplication is now not allowed to return until the goroutine it starts
cancels the context. Previously, it could return just before the context was
canceled which caused a race between its internal goroutine and
handlePrimaryStoreEvent which recreates a channel which that internal goroutine
was listening on.
handlePrimaryStoreEvent already correctly made sure that monitorReplication had
returned before recreating the channel, so proper handling of the sub-goroutine
of monitorReplication was all that was needed to avoid this race.
