<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div apple-content-edited="true">On 13 Jun 2014, at 07:03, W Chan <<a href="mailto:m4d.coder@gmail.com">m4d.coder@gmail.com</a>> wrote:</div><div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr"><div style="margin: 0in;"><font face="arial, helvetica, sans-serif">Design proposal for blueprint </font><a href="https://blueprints.launchpad.net/mistral/+spec/mistral-engine-executor-protocol">https://blueprints.launchpad.net/mistral/+spec/mistral-engine-executor-protocol</a></div><p style="margin:0in"></p><ul><li><span style="font-family:arial,helvetica,sans-serif">Rename Executor to Worker.</span><br></li></ul></div></blockquote>I’d be ok with Worker but would prefer ActionRunner since it reflects the purpose a little better although being more verbose.<br><blockquote type="cite"><div dir="ltr"><ul><li><span style="font-family:arial,helvetica,sans-serif">Continue to use RPC
server-client model in oslo.messaging for Engine and Worker protocol.</span><br></li></ul></div></blockquote>Sure.<br><blockquote type="cite"><div dir="ltr"><ul><li><span style="font-family:arial,helvetica,sans-serif">Use asynchronous call (cast)
between Worker and Engine where appropriate.</span><br></li></ul></div></blockquote>I would even emphasize: only async calls make sense.<br><blockquote type="cite"><div dir="ltr"><ul><li><span style="font-family:arial,helvetica,sans-serif">Remove any DB access from
Worker.</span><span style="font-family:arial,helvetica,sans-serif"> </span><span style="font-family:arial,helvetica,sans-serif">DB IO will only be done by
Engine.</span><br></li></ul></div></blockquote>I still have doubts it’s actually possible. This is a part of the issue I mentioned in the previous email. I’ll post more detailed email on that separately.<br><blockquote type="cite"><div dir="ltr"><ul><li><span style="font-family:arial,helvetica,sans-serif">Worker updates Engine that
it's going to start running action now.</span><span style="font-family:arial,helvetica,sans-serif">
</span><span style="font-family:arial,helvetica,sans-serif">If execution is not RUNNING and task is not IDLE, Engine tells
Worker to halt at this point.</span><span style="font-family:arial,helvetica,sans-serif">
</span><span style="font-family:arial,helvetica,sans-serif">Worker cannot assume execution state is RUNNING and task state is
IDLE because the handle_task message could have been sitting in the
message queue for awhile.</span><span style="font-family:arial,helvetica,sans-serif"> </span><span style="font-family:arial,helvetica,sans-serif">This call
between Worker and Engine is synchronous, meaning Worker will wait for a
response from the Engine.</span><span style="font-family:arial,helvetica,sans-serif">
</span><span style="font-family:arial,helvetica,sans-serif">Currently, Executor checks state and updates task state directly to
the DB before running the action.</span><br></li></ul></div></blockquote>Yes, that’s how it works now. First of all, like I said before we can’t afford making any sync calls between engine and executor because it’ll lead to problems with scalability and fault tolerance. So for that reason we make DB calls directly to make sure that execution and the task itself are in the suitable state. This would only work reliably for READ_COMMITTED transactions used in both engine and executor which I believe currently isn’t true since we use sqlite (it doesn’t seem to support them, right?). With mysql it should be fine.</div><div><br></div><div>So the whole initial idea was to use DB whenever we need to make sure that something is in a right state. That’s why all the reads should see only committed data. And we use queue just to notify executors about new tasks. Basically we could have even not used a queue and instead used db poll but with queue it looked more elegant.</div><div><br></div><div>It’s all part of one problem. Let’s try to figure out options to simplify the protocol and make it more reliable.<br><blockquote type="cite"><div dir="ltr"><ul><li><span style="font-family:arial,helvetica,sans-serif">Worker communicates result (success or failure) to Engine.</span><span style="font-family:arial,helvetica,sans-serif"> </span><span style="font-family:arial,helvetica,sans-serif">Currently, Executor is inconsistent and calls Engine.convey_task_result
on success and write directly to DB on failure.</span></li></ul></div></blockquote>Yes, that probably needs to be fixed.<br><br><blockquote type="cite"><div dir="ltr"><div style="margin: 0in;"><font face="arial, helvetica, sans-serif">Sequence</font></div><p style="margin:0in"></p><ol><li><span style="font-family:arial,helvetica,sans-serif">Engine ->
Worker.handle_task</span><br></li><li><span style="font-family:arial,helvetica,sans-serif">Worker converts action spec
to Action instance</span><br></li></ol></div></blockquote>Yes, it uses action spec in case if it’s ad-hoc action. If not, it just gets action class from the factory and instantiate it.<br><blockquote type="cite"><div dir="ltr"><ol start="3"><li><span style="font-family:arial,helvetica,sans-serif">Worker ->
Engine.confirm_task_execution. Engine returns an exception if execution
state is not RUNNING or task state is not IDLE.</span><br></li></ol></div></blockquote>Maybe I don’t entirely follow your thought but I think it’s not going to work. After engine confirms everything’s OK we’ll have a concurrency window again after that we’ll have to confirm the states again. That’s why I was talking about READ_COMMITTED DB transactions: we need to eliminate concurrency windows.<br><blockquote type="cite"><div dir="ltr"><ol start="4"><li><span style="font-family:arial,helvetica,sans-serif">Worker runs action</span><br></li><li><span style="font-family:arial,helvetica,sans-serif">Worker ->
Engine.convey_task_result</span></li></ol></div></blockquote>That looks fine (it’s as it is now). Maybe the only thing we need to pay attention to is to how we communicate errors back to engine. It seems logical that “convey_task_result()” can also be used to pass information about errors that that error is considered a special case of a regular result. Need to think it over though...<br><div><br></div></div><br><div><div>Renat Akhmerov</div><div>@ Mirantis Inc.</div></div><div><br></div></body></html>