Michael Kublin has uploaded a new change for review.

Change subject: engine: Adding support for lock to survive restart
......................................................................

engine: Adding support for lock to survive restart

The following patch will add the following functionality:
1. Introduced annottation parameter which should add a possibility not to 
relase in memory lock
   in the end of execute
2. By using this parameter we are enforcing command to release a lock only 
after that all
   child commands are finished
3. Added code which should solve restart problem - added acquireLockAsyncTask 
method

Change-Id: I8c73dac12f6f56ce92a16f94d3b49e0795ab2e46
Signed-off-by: Michael Kublin <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.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/EntityAsyncTask.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LockIdNameAttribute.java
5 files changed, 74 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/76/11976/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
index 0af39e4..ccce374 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
@@ -39,7 +39,7 @@
             creationInfo.setTaskType(AsyncTaskType.unknown);
         }
         AsyncTaskParameters asyncTaskParams = new 
AsyncTaskParameters(creationInfo, asyncTask);
-        return Construct(creationInfo.getTaskType(), asyncTaskParams);
+        return Construct(creationInfo.getTaskType(), asyncTaskParams, true);
     }
 
     /**
@@ -55,13 +55,13 @@
      *            the parameters by which we construct the task.
      * @return
      */
-    public static SPMAsyncTask Construct(AsyncTaskType taskType, 
AsyncTaskParameters asyncTaskParams) {
+    public static SPMAsyncTask Construct(AsyncTaskType taskType, 
AsyncTaskParameters asyncTaskParams, boolean duringInit) {
         try {
             SPMAsyncTask result = null;
             if (taskType == AsyncTaskType.unknown) {
                 result = new SPMAsyncTask(asyncTaskParams);
             } else {
-                result = new EntityAsyncTask(asyncTaskParams);
+                result = new EntityAsyncTask(asyncTaskParams, duringInit);
             }
             return result;
         }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
index 09c1dbb..4b20bbc 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
@@ -452,7 +452,7 @@
     }
 
     public SPMAsyncTask CreateTask(AsyncTaskType taskType, AsyncTaskParameters 
taskParameters) {
-        return AsyncTaskFactory.Construct(taskType, taskParameters);
+        return AsyncTaskFactory.Construct(taskType, taskParameters, false);
     }
 
     public synchronized void UpdateTaskWithActionParameters(Guid taskID, 
VdcActionParametersBase actionParameters) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
index 115840e..889fe30 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
@@ -317,7 +317,7 @@
                 getReturnValue().setCanDoAction(false);
             }
         } finally {
-            freeLock();
+            freeLockExecute();
         }
         return getReturnValue();
     }
@@ -426,18 +426,33 @@
         ExecutionHandler.startFinalizingStep(getExecutionContext());
 
         try {
+            initiateLockEndAction();
             setActionState();
             handleTransactivity();
             TransactionSupport.executeInScope(endActionScope, this);
         } catch (TransactionRolledbackLocalException e) {
             log.infoFormat("EndAction: Transaction was aborted in {0}", 
this.getClass().getName());
         } finally {
+            freeLock();
             if (getCommandShouldBeLogged()) {
                 logCommand();
             }
         }
 
         return getReturnValue();
+    }
+
+    /**
+     * The following method should initiate a lock , in order to release it at 
endAction()
+     */
+    private void initiateLockEndAction() {
+        if (commandLock == null) {
+            LockIdNameAttribute annotation = 
getClass().getAnnotation(LockIdNameAttribute.class);
+            if (annotation != null && !annotation.isReleaseExecute()) {
+                commandLock = buildLock();
+            }
+
+        }
     }
 
     private void handleTransactivity() {
@@ -490,6 +505,7 @@
             exceptionOccurred = true;
             throw e;
         } finally {
+            freeLock();
             if (TransactionSupport.current() == null) {
 
                 // In the unusual case that we have no current transaction, 
try to cleanup after yourself and if the
@@ -1124,7 +1140,6 @@
             compensate();
         } finally {
             try {
-                freeLock();
                 if (getCommandShouldBeLogged()) {
                     logCommand();
                 }
@@ -1403,6 +1418,8 @@
         }
     }
 
+    private boolean isReleaseExecute = true;
+
     /**
      * Object which is representing a lock that some commands will acquire
      */
@@ -1420,6 +1437,7 @@
         LockIdNameAttribute annotation = 
getClass().getAnnotation(LockIdNameAttribute.class);
         boolean returnValue = true;
         if (annotation != null) {
+            isReleaseExecute = annotation.isReleaseExecute();
             if (!annotation.isWait()) {
                 returnValue = acquireLockInternal();
             } else {
@@ -1429,13 +1447,27 @@
         return returnValue;
     }
 
+    /**
+     * The following method should be called after restart of engine during 
initialization of asynchronous task
+     * @return
+     */
+    public final boolean acquireLockAsyncTask() {
+        LockIdNameAttribute annotation = 
getClass().getAnnotation(LockIdNameAttribute.class);
+        boolean returnValue = true;
+        if (annotation != null) {
+            isReleaseExecute = annotation.isReleaseExecute();
+            if (!isReleaseExecute) {
+                returnValue = acquireLockInternal();
+            }
+        }
+        return returnValue;
+    }
+
     protected boolean acquireLockInternal() {
         // if commandLock is null then we acquire new lock, otherwise probably 
we got lock from caller command.
         if (commandLock == null) {
-            Map<String, Pair<String, String>> exclusiveLocks = 
getExclusiveLocks();
-            Map<String, Pair<String, String>> sharedLocks = getSharedLocks();
-            if (exclusiveLocks != null || sharedLocks != null) {
-                EngineLock lock = new EngineLock(exclusiveLocks, sharedLocks);
+            EngineLock lock = buildLock();
+            if (lock != null) {
                 Pair<Boolean, Set<String>> lockAcquireResult = 
getLockManager().acquireLock(lock);
                 if (lockAcquireResult.getFirst()) {
                     log.infoFormat("Lock Acquired to object {0}", lock);
@@ -1450,6 +1482,16 @@
         return true;
     }
 
+    private EngineLock buildLock() {
+        EngineLock lock = null;
+        Map<String, Pair<String, String>> exclusiveLocks = getExclusiveLocks();
+        Map<String, Pair<String, String>> sharedLocks = getSharedLocks();
+        if (exclusiveLocks != null || sharedLocks != null) {
+            lock = new EngineLock(exclusiveLocks, sharedLocks);
+        }
+        return lock;
+    }
+
     private void acquireLockAndWait() {
         // if commandLock is null then we acquire new lock, otherwise probably 
we got lock from caller command.
         if (commandLock == null) {
@@ -1462,6 +1504,12 @@
         }
     }
 
+    private void freeLockExecute() {
+        if (isReleaseExecute || !getSucceeded()) {
+            freeLock();
+        }
+    }
+
     protected void freeLock() {
         if (commandLock != null) {
             getLockManager().releaseLock(commandLock);
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java
index fb4715c..fde815d 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java
@@ -37,9 +37,7 @@
     private static EntityMultiAsyncTasks 
GetEntityMultiAsyncTasksByContainerId(Object containerID) {
         EntityMultiAsyncTasks entityInfo = null;
         synchronized (_lockObject) {
-            if (_multiTasksByEntities.containsKey(containerID) && 
_multiTasksByEntities.get(containerID) != null) {
-                entityInfo = _multiTasksByEntities.get(containerID);
-            }
+            entityInfo = _multiTasksByEntities.get(containerID);
         }
 
         return entityInfo;
@@ -49,13 +47,25 @@
         return GetEntityMultiAsyncTasksByContainerId(getContainerId());
     }
 
-    public EntityAsyncTask(AsyncTaskParameters parameters) {
+    public EntityAsyncTask(AsyncTaskParameters parameters, boolean duringInit) 
{
         super(parameters);
+        boolean isNewCommanAdded = false;
         synchronized (_lockObject) {
             if (!_multiTasksByEntities.containsKey(getContainerId())) {
                 log.infoFormat("EntityAsyncTask::Adding EntityMultiAsyncTasks 
object for entity '{0}'",
                         getContainerId());
                 _multiTasksByEntities.put(getContainerId(), new 
EntityMultiAsyncTasks(getContainerId()));
+                isNewCommanAdded = true;
+            }
+        }
+        if (duringInit && isNewCommanAdded) {
+            CommandBase<?> command =
+                    
CommandsFactory.CreateCommand(parameters.getDbAsyncTask().getaction_type(),
+                            
parameters.getDbAsyncTask().getaction_parameters());
+            if (!command.acquireLockAsyncTask()) {
+                log.warnFormat("Failed to acquire locks for command {0} with 
parameters {1}",
+                        parameters.getDbAsyncTask().getaction_type(),
+                        parameters.getDbAsyncTask().getaction_parameters());
             }
         }
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LockIdNameAttribute.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LockIdNameAttribute.java
index d751154..a4e25b8 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LockIdNameAttribute.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LockIdNameAttribute.java
@@ -9,4 +9,6 @@
 @Retention(RetentionPolicy.RUNTIME)
 public @interface LockIdNameAttribute {
     boolean isWait() default false;
+
+    boolean isReleaseExecute() default true;
 }


--
To view, visit http://gerrit.ovirt.org/11976
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c73dac12f6f56ce92a16f94d3b49e0795ab2e46
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to