I think this will improve the transaction response messages Kuba
mentioned where it was an empty transaction instead of a nil or not
there... if not it should make it easier to do that anyhow.
also adds a "noSleep" option to the server command to avoid the 5
second sleep we introduced on startup for non-coordinator cluster
nodes. The sleep doesn't seem to be needed in the tests and makes them
much slower.
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`
This PR adds support for anti-entropy syncing for integer
and decimal fields. It differs from the logic for other
field types in that it does not rely on a consensus to determine
what the value should be; instead, it considers the correct
values to be those of the primary replica. From there, data
is pushed to all non-primary replicas.
This PR adds a translationSyncer interface; I tried to include
comments in the code explaining what's going on. This is taken
from those comments:
translationSyncer provides an interface allowing a function
to notify the server that an action has occurred which requires
the translation sync process to be reset. In general, this
includes anything which modifies schema (add/remove index, etc),
or anything that changes the cluster topology (add/remove node).
I originally considered leveraging the broadcaster since that was
already in place and provides similar event messages, but the
broadcaster is really meant for notifiying other nodes, while
this is more akin to an internal message bus. In fact, I think
a future iteration on this may be to make it more generic so
it can act as an internal message bus where one of the messages
being published is "translationSyncReset".
In the case where a field with a foreign index opens before the
foreign index has opened (and is available as a reference in the
holder), push the field into a queue to have its foreign index
applied once all indexes have opened.
The snapshot queue needs a bit more subtlety. In some cases,
we really do want to do a snapshot right now -- these shouldn't
have to wait for possibly a hundred or more other snapshots
to complete.
In other cases, we don't really care that much whether we do
a snapshot, and just dropping it is probably fine.
To accommodate this, we distinguish between "urgent" and
"normal" snapshots, and between "Immediate" (does an urgent
snapshot, waits for it) and "Enqueue" (might enqueue a snapshot
but *also might not* if we're already busy). There's a
corresponding "Await" to wait for a snapshot, if one is
pending, but not if one isn't.
We also have a background scan that checks the holder. It will
scan pretty actively when it's finding fragments that need
snapshots (no enqueued snapshot, opN > MaxOpN). It pauses
for a second after every hundred fragments that didn't need
snapshots, and for a minute after each holder scan that didn't
find any. So, if you don't need snapshots, it does basically
nothing, if you do, it'll be moderately aggressive about
submitting tasks -- but it always waits if there's *any*
requested snapshots in the queues.
Updates since initial draft:
Check results from Await more consistently, and in one case, use Immediate
instead and then check its error.
Fix a race condition. The race condition comes about if:
1. You have a limited enough worker pool that this can happen.
(In testing we tend to have a worker pool of 1.)
2. A fragment is in the normal, non-urgent, queue already.
3. An immediate request comes in for that fragment. This always
happens *with the fragment lock held*.
4. A worker thread grabs that fragment from the queue.
5. The worker thread now waits on the lock. Meanwhile, the
immediate request blocks on sending the fragment to the urgent
queue.
6. The worker can't read the urgent queue, and the immediate
request can't send it, so the immediate request can't proceed.
What's supposed to happen is that the immediate request sends
the thing, and gets into Await(), which sleeps on a condition
variable using the lock, which is to say, releases the lock.
The obvious resolution is to let go of the lock, send the
message, and then reclaim the lock. But then we have the
possibility that the message sent ends up with a timestamp
right after a snapshot that happened *after* the Immediate
request was started. Oops. So we create the request, then let
go of the lock, then send the request, then reclaim the lock
and go into the Await state. All is well.
This is on top of more general use of wait groups, etcetera,
to allow us to ensure that any holder scans terminate *before*
we close the channels they might otherwise be trying to write to.
So, shutdown process is now:
* grab lock on queue (workers and scanners don't use the lock)
* mark snapshotqueue done
* wait for holder scans to complete/exit
* close and nil out all the channels
* release lock
Anything trying to submit to this needs to hold the lock, unless
it's a holder scan, so either it got the lock before we did and already
submitted the thing, or it will get the lock after this and not find
a channel to write to; it's just the holder scanner that has an
ongoing thing that might have started a write to the channel *without*
a lock held, because it's expected that it might have to wait minutes
or hours before the write will complete because it's a background task.
Also, rework the background holder scan to grab lists of
indexes/fields/views/fragments, then scan the grabbed/copied lists,
rather than iterating over maps, allowing us to grab the lock when
we're about to access a thing and let it go when done.
There might be a simpler/cleaner way to do this but opinions on how
safe it is are very mixed, so in the mean time, I'm making the range
behavior not depend at all on there being no writes to the various tiers
of holder/index/view/fragment during the background scans.
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.
The attempt to fix up the logic broke returns from loadNodeID()
in some cases, because it was overwriting the node ID generated
in the IsNotExist case.
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.
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.
