1 commit

Author	SHA1	Message	Date
Seebs	c5136b14db	ensmarten snapshot queue ... 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.	2019-11-12 12:15:13 -06:00