This adopts the task pool functionality to let us spawn new worker
threads when worker threads are blocked. The underlying reason for
this is the same as the reason for the previous worker-pool-growing
strategy; while our design persistently has at least one thing which
can proceed, it can be the case that there are N things blocked,
where N is the size of our worker pool. Blocked workers shouldn't
count against our desired number of workers.
Originally, the intent was to thread this into RBF, and provide
backpressure from RBF on the pool when blocking on writes. Unfortunately,
that's not good enough, because while a write is blocked, the Qcx
calling it is *also* holding the Qcx's mutex, which means that any other
NewTx on that Qcx will *also* block. So we need to block for the
entire time of the NewTx.
Removing the existing worker spawning code resulted in a subtle
and maybe-harmless change; prior to this, each invocation of `mapperLocal`
would hold a lock, which meant that all the tasks for a given local mapper
would be put in the queue *sequentially*, ensuring that they'd all be
picked up by workers before things from later workers.
With the new pushback, that's not, strictly, necessary. Also, if you
disable it, you can end up with 300,000 goroutines at once, most of them
blocked.
A smallish run does, in fact, eventually complete anyway -- it will
indeed keep making workers until everything gets one. However, while
it's *correct*, it's also noticably *slower*. The same test workload
goes from around 33 seconds to a bit over 40 seconds when that lock
isn't present. (But that's with an extremely small WAL write cap
introduced to make the previous deadlock possible.)
With large numbers of shards, the practical impact is that you can
have quite a lot of things in process, with hundreds of goroutines
each, all blocked waiting for one writer. If we force them to all be
processed at the same time, all the reads that are connected to
each other are much more likely to get all processed at once, before
something new comes along.
In short, that lock isn't strictly necessary but it seems to help
noticably with performance and reduce simultaneous goroutines
significantly.
if we have more than twice our starting worker pool, and have had no
tasks when checking the queue for multiple rounds, send a job telling
the system to retire a worker. eventually we'll get down to about 2x
the starting pool size if we stay idle.
When a qcx is a write, every Tx under it closes immediately, thus
invalidating all returned data. Thus, if you do a Not() inside a Store(),
you're doing a difference on an existence row and some other row
call... and both of those rows were run, individually, as separate
transactions that got invalidated the moment they were fetched. Oops.
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
```
The actual code here is mostly jaffee's, but I've reworked it some.
This doesn't directly seem to be using UnionInPlace, but really it
is.
The actual logic inside (*Row).Union is a mess and probably silly
in a few ways, but hardly matters. The important part is that,
instead of calling it once per child as we get them, we gather
all of them at once and then call it on all of them. That gets
us a call to (*Row).Union that does a very elaborate dance to
compute a call to (*rowSegment).Union on the only segment present
in each of those rows, which then does a simpler thing to
call (*Bitmap).Union() with the first response as a receiver
and the rest as parameters, and THAT then ends up calling either
unionIntoTargetSingle() if there's only one other bitmap,
or using UnionInPlace on a Freeze() of the first bitmap, which
gets us (we hope) the benefits of the fancy UnionInPlace logic.
Every part of this is a reminder that we really need to replace
roaring and also the Row/rowSegment stuff some day.
staticcheck points out that the break is otherwise an ineffective
break because it just ends the current case clause of the switch
it's in, which is true.
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.
The GetTx logic is deeply broken, this DOES NOT fix the underlying
bug.
When any call anywhere in a given set of calls has a top-level write,
we perform all transactions as write transactions, and we do not cache or
share those transactions. This means that anything which causes a
second GetTx for the same index/shard deadlocks against itself.
The two easy to find cases by casual inspection are time quantums
and Not queries, so this addresses those, but this should NOT be
considered a general fix.
