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.
The new roaring iterator used for the remap and importroaring
things could also be used for unmarshalling roaring streams,
and it's a slightly simpler design that doesn't need two passes
through the data. This patch cleans that up a bit, makes it work
better with ops logs, and uses that instead. It appears to
noticably but not immensely reduce the time imports take, but it
also gets us back down to one thing parsing roaring formats.
There are a couple of subtle changes to errors we were testing
for in various tests, and one of the fuzz tests goes away because
it was actually itself an erroneous error message -- it was reporting
the header of a roaring file as an invalid op because the op log
reader was running on the header for roaring files with zero
containers. Oops.
we are experiencing issues with CI where it fails with race: limit on
8128 simultaneously alive goroutines is exceeded, dying
this, despite the fact that closing the executor should clean up all
worker goroutines. Apparently in CircleCI runtime.NumCPU() reports 36,
so the goroutines added up quickly.
the size of the work chan probably doesn't matter... there is some
discussion of this on the associated PR
https://github.com/pilosa/pilosa/pull/2034
may test with an unbuffered channel as well.
Closing the executor avoids leaking goroutines which seems to be an
issue while running the test suite.
Pilosa previously spawned a goroutine for each remote node that a
query needed to be forwarded to, and then forwarded a single request
containing all the shards that the query should operate on. It then
spawned a goroutine *per local shard* to process the query
locally. This was fine if there weren't too many shards, or too many
queries coming in concurrently, but we found that it created issues
when there were 100s or 1000s of shards per node, and dozens of
queries arriving concurrently.
Specifically, the memberlist "hiccup" issue is highly correlated with
many goroutine scenarios, and after applying this patch, memberlist
complaints in the logs were much decreased, and nodeLeave events under
concurrent query load almost entirely eliminated.
This patch creates a fixed size pool of goroutines to do local shard
processing, and passes work to them through a channel, one job per
query per shard. Handling of remote requests (forwarding queries) is
unchanged.
We set the pool size to NumCPU()+8 somewhat arbitrarily, but this
seemed to work pretty well in our testing on 32 core machines. It's a
pretty big improvement over launching a goroutine per shard per query
which is what we were doing previously, so we can tune it more later
if necessary.
Have found some potential performance or stability issues associated
with lots of mutex blocking in getting a parent span's context. Want
the ability to totally disable tracing to help debugging.
