cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rafaelweingartner <...@git.apache.org>
Subject [GitHub] cloudstack issue #1935: CLOUDSTACK-9764: Delete domain failure due to Accoun...
Date Tue, 21 Feb 2017 21:55:56 GMT
Github user rafaelweingartner commented on the issue:

    https://github.com/apache/cloudstack/pull/1935
  
    @nvazquez great work.
    However, there is a catch there that I think you might have overlooked. This problem is
caused by the method extraction I suggested.
    
    If you take a look at the code before the extraction, every time that an exception is
thrown, the code was setting the variable `rollBackState = true`. This happens at lines 287,
305, and 313. Now that the code was extracted, setting those variables to `true` does not
work anymore, because of the context those variables are declared change.
    
    In my opinion, this code was kind of weird before. It was throwing an exception that is
caught right away and setting a control variable to be executed on `finally` block. The only
reason I see for this is that if other exceptions that are not the ones generated at lines
292, 310, and 325 happen, and we do not want to execute the rollback for them. However, this
seems error prone, leading to database inconsistencies.
    
    I would change the "rollback" code (lines 342-345) to the catch block.
    
    I do not know if I have been clear, we can discuss this further. I may have overlooked
some bits of it as well (it is a quite complicated bit of code).



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message