Re: [openstack-dev] [Mistral] task update at end of handle_task in executor
I agree with all the explanations given here. > On Thu, Mar 27, 2014 at 9:47 AM, Manas Kelshikar wrote: > Yes. It is a bug and should be done before line 119: > self._do_task_action(db_task). It can definitely lead to bugs especially > since _do_task_action itself updates the status. > > On Wed, Mar 26, 2014 at 8:46 PM, W Chan wrote: > In addition, for sync tasks, it'll overwrite the task state from SUCCESS to > RUNNING. Guys, good catch. Has it been addressed yet? Renat Akhmerov @ Mirantis Inc. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Mistral] task update at end of handle_task in executor
> > In case of async tasks, executor keeps the task status at RUNNING, and a > 3rd party system will call convey_task_resutls on engine. Yes, it is correct. With this approach (in case sync task), we set task state to SUCCESS if it returns a result, or ERROR if we can't see a result and exception is raised. It is a bug and should be done before line 119: self._do_task_action( > db_task). And also lines 120-123 ( https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L120-L123) are incorrect since in _do_task_action updates the task state. But we have two different types of task (async, sync) and I think we should update task state to RUNNING before invoking _do_task_action and remove this lines - https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L58-L61 . On Thu, Mar 27, 2014 at 9:47 AM, Manas Kelshikar wrote: > Yes. It is a bug and should be done before line 119: self._do_task_action > (db_task). It can definitely lead to bugs especially since > _do_task_action itself updates the status. > > > > > On Wed, Mar 26, 2014 at 8:46 PM, W Chan wrote: > >> In addition, for sync tasks, it'll overwrite the task state from SUCCESS >> to RUNNING. >> >> >> On Wed, Mar 26, 2014 at 8:41 PM, Dmitri Zimine wrote: >> >>> My understanding is: it's the engine which finalizes the task results, >>> based on the status returned by the task via convey_task_result call. >>> >>> >>> https://github.com/stackforge/mistral/blob/master/mistral/engine/abstract_engine.py#L82-L84 >>> >>> https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L44-L66 >>> >>> In case of async tasks, executor keeps the task status at RUNNING, and a >>> 3rd party system will call convey_task_resutls on engine. >>> >>> >>> https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123 >>> , >>> >>> >>> This line however looks like a bug to me: at best it doesnt do much and >>> at worst it overwrites the ERROR previously set in here >>> http://tinyurl.com/q5lps2h >>> >>> Nicolay, any better explanation? >>> >>> >>> DZ> >>> >>> On Mar 26, 2014, at 6:20 PM, W Chan wrote: >>> >>> Regarding >>> https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123, >>> should the status be set to SUCCESS instead of RUNNING? If not, can >>> someone clarify why the task should remain RUNNING? >>> >>> Thanks. >>> Winson >>> >>> ___ >>> OpenStack-dev mailing list >>> OpenStack-dev@lists.openstack.org >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>> >>> >>> >>> ___ >>> OpenStack-dev mailing list >>> OpenStack-dev@lists.openstack.org >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>> >>> >> >> ___ >> OpenStack-dev mailing list >> OpenStack-dev@lists.openstack.org >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > -- Best Regards, Nikolay ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Mistral] task update at end of handle_task in executor
Yes. It is a bug and should be done before line 119: self._do_task_action( db_task). It can definitely lead to bugs especially since _do_task_action itself updates the status. On Wed, Mar 26, 2014 at 8:46 PM, W Chan wrote: > In addition, for sync tasks, it'll overwrite the task state from SUCCESS > to RUNNING. > > > On Wed, Mar 26, 2014 at 8:41 PM, Dmitri Zimine wrote: > >> My understanding is: it's the engine which finalizes the task results, >> based on the status returned by the task via convey_task_result call. >> >> >> https://github.com/stackforge/mistral/blob/master/mistral/engine/abstract_engine.py#L82-L84 >> >> https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L44-L66 >> >> In case of async tasks, executor keeps the task status at RUNNING, and a >> 3rd party system will call convey_task_resutls on engine. >> >> >> https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123 >> , >> >> >> This line however looks like a bug to me: at best it doesnt do much and >> at worst it overwrites the ERROR previously set in here >> http://tinyurl.com/q5lps2h >> >> Nicolay, any better explanation? >> >> >> DZ> >> >> On Mar 26, 2014, at 6:20 PM, W Chan wrote: >> >> Regarding >> https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123, >> should the status be set to SUCCESS instead of RUNNING? If not, can >> someone clarify why the task should remain RUNNING? >> >> Thanks. >> Winson >> >> ___ >> OpenStack-dev mailing list >> OpenStack-dev@lists.openstack.org >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> >> >> ___ >> OpenStack-dev mailing list >> OpenStack-dev@lists.openstack.org >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Mistral] task update at end of handle_task in executor
In addition, for sync tasks, it'll overwrite the task state from SUCCESS to RUNNING. On Wed, Mar 26, 2014 at 8:41 PM, Dmitri Zimine wrote: > My understanding is: it's the engine which finalizes the task results, > based on the status returned by the task via convey_task_result call. > > > https://github.com/stackforge/mistral/blob/master/mistral/engine/abstract_engine.py#L82-L84 > > https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L44-L66 > > In case of async tasks, executor keeps the task status at RUNNING, and a > 3rd party system will call convey_task_resutls on engine. > > > https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123 > , > > > This line however looks like a bug to me: at best it doesnt do much and > at worst it overwrites the ERROR previously set in here > http://tinyurl.com/q5lps2h > > Nicolay, any better explanation? > > > DZ> > > On Mar 26, 2014, at 6:20 PM, W Chan wrote: > > Regarding > https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123, > should the status be set to SUCCESS instead of RUNNING? If not, can > someone clarify why the task should remain RUNNING? > > Thanks. > Winson > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Mistral] task update at end of handle_task in executor
My understanding is: it's the engine which finalizes the task results, based on the status returned by the task via convey_task_result call. https://github.com/stackforge/mistral/blob/master/mistral/engine/abstract_engine.py#L82-L84 https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L44-L66 In case of async tasks, executor keeps the task status at RUNNING, and a 3rd party system will call convey_task_resutls on engine. > https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123, This line however looks like a bug to me: at best it doesnt do much and at worst it overwrites the ERROR previously set in here http://tinyurl.com/q5lps2h Nicolay, any better explanation? DZ> On Mar 26, 2014, at 6:20 PM, W Chan wrote: > Regarding > https://github.com/stackforge/mistral/blob/master/mistral/engine/scalable/executor/server.py#L123, > should the status be set to SUCCESS instead of RUNNING? If not, can someone > clarify why the task should remain RUNNING? > > Thanks. > Winson > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
