#791195

Jan Provazník jprovazn at redhat.com
Fri Feb 17 09:23:16 UTC 2012


On 02/16/2012 11:06 PM, Matt Wagner wrote:
> This was intended to be a comment on
> https://bugzilla.redhat.com/show_bug.cgi?id=791195 , but Bugzilla is
> currently down. I started to look at this, but didn't get too far.
>
> Here is the comment I tried to leave:
>
> I started to take a look at this, but I'm mildly puzzled and I wonder if
> the potential refactor can go deeper.
>
> It looks like Taskomatic's "destroy_instance" doesn't actually have any
> sort of retry logic, nor do I even see it happening in the background.
>
> If I remove the retry logic, destroy_on_provider is left like this:
>
> if<really long conditional>
>           @task = self.queue_action(self.owner, 'destroy')
>          raise I18n.t"instance.errors.cannot_destroy" unless @task
>          Taskomatic.destroy_instance(@task)
> end
>
> queue_action just creates an Event and a Task object in the database and
> returns the task.
>
> We then call Taskomatic.destroy_instance, which updates some metadata on
> @task and then calls "destroy!" on the instance via Deltacloud. If that
> fails, we update the task and return.
>
> If the 500 retries existed solely to guard against the task not existing
> (!), then we can indeed drop it. But I rather assumed it was meant to
> guard against API errors as well.
>

The taskomatic's destroy_instance method is whole wrapped in a rescue 
block (except task.save! call in ensure block) so 500-retries block 
doesn't guard against API errors anyway (if it was the goal, then it 
should check task.state too).

> Re: #3, I'm not sure I understand. If we just moved it to after_update,
> we'd delete the instance on the provider any time its state changed. I
> think the current before_destroy hook is correct. (Though we do have an
> instance_observer we could use.)
>
>

Sorry, I described it wrong. If retries block is there because of doing 
busy-wait loop until api destroy call succeeds, it probably means that 
it tries to call destroy method too early when the instance is not ready 
to be destroyed yet - this could be because a check of instance state 
change in instance_observer is wrong and we trigger destroy action too 
early or there is a weird bug on dc-core/rhevm side and it doesn't allow 
destroy an instance even if it should. I hope it's first option and we 
have a wrong check of instance state in instance_observer.

Tomas: do you remember what was the reason of retry block and how 
deletion of instances works on RHEVM?

Jan



More information about the aeolus-devel mailing list