Replying off-list to follow your lead, but this might be a good
discussion to have upstream where Stefan and Jeff can see it.
On 09/26/2016 11:53 AM, Vladimir Sementsov-Ogievskiy wrote:
Hi John!
What about this series? If you are working on this, I can tell about
another related problem.
I got mired in trying to think about how to delete a BlockJob that
hasn't technically been started yet -- this patch series is unsafe in
that it will be unable to "cancel" a job that hasn't started yet.
(And I don't want to start a job just to cancel it, that's gross.)
I need to expand the job modifications here a little bit to make this
completely safe. I may need to create a job->started bool that allows
you to call a job_delete() function if and only if started is false.
I'll try to send this along this week.
I'm working on async scheme for backup. I've started from something like
just wrapping backup_do_cow into coroutine and use this coroutine in
full-backup-loop instead of just call backup_do_cow. This simple
approach seems better than used in mirror - in mirror we create new
coroutine for each async read and write, when I create new coroutine for
copy of = read + write. But then I decided to rewrite this to
worker-coroutines scheme. I.e. we do not create new coroutine for each
request and control their count by " while (job->inflight_requests >=
MAX_IN_FLIGHT) { yield }" but we create MAX_IN_FLIGHT worker coroutines
at backup start, and then they just copy cluster by cluster, using
common cop_bitmap to synchronize. All cluster copying is done in
backup-workers and write-notifiers only change the order in which
workers copy clusters. And I liked it all until I figured out the error
handling.
Makes sense to me so far.
I can't handle errors in workers, because I need to call
block_job_error_action, which may stop blockjob, and it seems to be a
problem if several workers calls it simultaneously. So, worker have to
What kind of problems are you seeing? Maybe we can fix them? is it for
STOP cases? (or also on IGNORE/REPORT?)
Otherwise, just stash an error object in a location that the master
coroutine can find, pause/terminate all worker threads, and signal the
error.
defer error handling to main coroutine of the blockjob (this coroutine
in my solution doesn't do copying but only handles blockjob staff like
pauses, stops, throttling). So, we will have a queue of workers, waiting
for main coroutine to handle their errors. This looks too complicated..
Really? I don't think it's too bad. The worker detects an error and
creates a record of the problem, then the worker yields.
The controller detects the worker has yielded and sees the record of
the error. The controller opts not to re-schedule the worker.
The controller then instructs all remaining workers to yield. Once all
workers are yielded, the controller invokes block_job_error_action.
From there, we can handle the error accordingly.
Either way it sounds like we're going to have to manage the complexity
of what happens when a job with worker coroutines is paused by e.g.
the QMP monitor: the controller needs to send and coordinate messages
to the workers; so this doesn't sound like too much added complexity
unless I'm dreaming.
And finally I come to the idea that all this problems and complications
are because blockjob is not created for asynchronous work, because
blockjob has only one working coroutine. So the true way is to maintain
several working coroutines on generic blockjob layer. This will lead to
simpler and transparent async schemes for backup and mirror (and may be
other blockjobs).
So, the question is: what about refactoring blockjobs, to move from one
working coroutine to several ones? Is it hard and is it possible? I do
not understand all block-jobs related staff..
I think jobs will need to remain "one coroutine, one job" for now, but
there's no reason why drive-backup or blockdev-backup can't just
create multiple jobs each if that's what they need to do. (The backup
job object could, in theory, just have another job pointer to a helper
job if it really became necessary.)
Let's try to solve your design problem first and see if we can't make
error reporting behave nicely in a contested environment.