Liron Aravot has uploaded a new change for review.
Change subject: core: deadlock when creating diskless snapshots.
......................................................................
core: deadlock when creating diskless snapshots.
When creating a new diskless snapshots, endsuccesfully method is being
called from the execute() method as there are no disks, thus - there are
no async tasks created. The command is being executed within
transaction and performs db updates which references FK, meaning lock on
the referenced rows is acquired in the db. When a new transaction is opened
and there's an attempt to update the same rows records- deadlock occurs:
transaction A -> update db record (FK triggered locks acuired) -> row lock ->
waits for transaction B.
transactio B -> update db record -> waits for row lock which is acquired
by transaction A.
Change-Id: I2ca72a2b7b881827493ddec5c53bd2c485a74084
Signed-off-by: Liron Aravot <lara...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
2 files changed, 25 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/57/10857/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
index f5d0c45..5d905bd 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
@@ -85,6 +85,10 @@
return selectedActiveDisks;
}
+ private void incrementVmGeneration() {
+ getVmStaticDAO().incrementDbGeneration(getVm().getId());
+ }
+
@Override
protected void executeVmCommand() {
Guid newActiveSnapshotId = Guid.NewGuid();
@@ -93,19 +97,30 @@
getParameters().setInitialVmStatus(getVm().getStatus());
getSnapshotDao().updateId(createdSnapshotId, newActiveSnapshotId);
- new SnapshotsManager().addSnapshot(createdSnapshotId,
- getParameters().getDescription(),
- getParameters().getSnapshotType(),
- getVm(),
- getCompensationContext());
- freeLock();
setActionReturnValue(createdSnapshotId);
if (getDisksList().isEmpty()) {
getParameters().setTaskGroupSuccess(true);
- endSuccessfully();
+ new SnapshotsManager().addSnapshot(createdSnapshotId,
+ getParameters().getDescription(),
+ SnapshotStatus.OK,
+ getParameters().getSnapshotType(),
+ getVm(),
+ true,
+ getCompensationContext());
+ // at the moment there's no need to execute vdsm Snapshot command
for diskless snapshots,
+ // when support for ram snapshot will be introduced, this vdsm
command should be executed
+ // for diskless snapshots as well (currently executed within
endSuccesfully() method.
+ incrementVmGeneration();
} else {
+ new SnapshotsManager().addSnapshot(createdSnapshotId,
+ getParameters().getDescription(),
+ getParameters().getSnapshotType(),
+ getVm(),
+ getCompensationContext());
+
+ freeLock();
for (DiskImage image : getDisksList()) {
ImagesActionsParametersBase tempVar = new
ImagesActionsParametersBase(image.getImageId());
tempVar.setDescription(getParameters().getDescription());
@@ -134,9 +149,8 @@
"CreateAllSnapshotsFromVmCommand::executeVmCommand: Failed to create
snapshot!");
}
}
-
- setSucceeded(true);
}
+ setSucceeded(true);
}
/**
@@ -168,7 +182,7 @@
revertToActiveSnapshot(createdSnapshotId);
}
- getVmStaticDAO().incrementDbGeneration(getVm().getId());
+ incrementVmGeneration();
endActionOnDisks();
setSucceeded(taskGroupSucceeded);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
index adb3b05..1265165 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
@@ -114,7 +114,7 @@
* @param compensationContext
* In case compensation is needed.
*/
- protected Snapshot addSnapshot(Guid snapshotId,
+ public Snapshot addSnapshot(Guid snapshotId,
String description,
SnapshotStatus snapshotStatus,
SnapshotType snapshotType,
--
To view, visit http://gerrit.ovirt.org/10857
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ca72a2b7b881827493ddec5c53bd2c485a74084
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
