Change in ovirt-engine[master]: core: add 'completed' method to RollbackHandler

mperina at redhat.com mperina at redhat.com
Mon Jan 11 08:28:49 UTC 2016


Martin Peřina has submitted this change and it was merged.

Change subject: core: add 'completed' method to RollbackHandler
......................................................................


core: add 'completed' method to RollbackHandler

motivation: there's missing onComplete method allowing to do some
actions only when transaction completed.

Current usecase: 'client' returns MAC address back to MAC address
pool. After doing that, rollback can occur and in that case we need
to put that MAC address back to pool. Simple 'onRollback' listener
seems to be sufficient for that at first, but we need to realize,
that putting MAC address back to pool makes it immediately available
for other threads, which means we may encounter problems in on
rollback handler if someone used this MAC address meantime. So correct
solution here would be, when returning MAC address back to pool, pool
adds this MAC address to some collection to be returned, and does so
only when related transaction succeeded.

Fixes:
• CommandBase implements RollbackHandler. This is from OO point of
view wrong, since command is not a RollbackHandler.
• Previous point also touches another issue — having method 'rollback'
directly on Command class, declared as 'non final' means that one
can easily make error and suppress logging in CommandBase.rollback
method while overriding it to add some custom handling. This should be
encapsulated, no one should have possibility to tinker with
CommandBase RollbackHandler. Multiple handler can be registered, so
every command requiring it's own code in rollback handler, should
declare and register his own. In case where order of handlers matter,
we can defined composite RollbackHandler, but I do not expect
necessity of doing so in near future.

Bug-Url: https://bugzilla.redhat.com/1283062
Change-Id: I352be763c84b2c3e640dbc23d15a5a294a6193e1
Signed-off-by: Martin Mucha <mmucha at redhat.com>
---
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddMacPoolCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMacPoolCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateMacPoolCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/RemoveSnapshotSingleDiskCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/cinder/RemoveCinderSnapshotDiskCommand.java
A backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/NoOpTransactionCompletionListener.java
D backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/RollbackHandler.java
A backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionCompletionListener.java
M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionSupport.java
M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VDSCommandBase.java
M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/AddNetworkVDSCommand.java
12 files changed, 185 insertions(+), 119 deletions(-)

Approvals:
  Martin Mucha: Verified
  Jenkins CI: Passed CI tests
  Moti Asayag: Looks good to me, approved



-- 
To view, visit https://gerrit.ovirt.org/48817
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I352be763c84b2c3e640dbc23d15a5a294a6193e1
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmucha at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Mucha <mmucha at redhat.com>
Gerrit-Reviewer: Martin Peřina <mperina at redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>



More information about the Engine-commits mailing list