Arik Hadas has posted comments on this change.
Change subject: core: increase AutoStartVmsRunner job frequency
......................................................................
Patch Set 8:
(1 comment)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoStartVmsRunner.java
Line 51:
Line 52: autoStartVmsToRun.removeAll(idsToRemove);
Line 53: }
Line 54:
Line 55: private EngineLock createLockForRunVmCommand(Guid vmId) {
I understand the advantage of such change as we won't need to change it here if
the locks for RunVm change.
But there's a trade-off because I don't know if we have proper place to hold it
(VmHandler?), it will be static thus it will be harder to access command's
properties for new locks (if necessary) and the message attached to the lock
will have to be the same (or given as a parameter as well).
I can think of two better alternatives:
1. to extract the list creations (for exclusive and shared locks) to static
methods in RunVmCommandBase and call it from here.
2. add RunVmLock which is an object (that can extend EngineLock) which
represent the locks that are needed in those places. in both places we'll do
new RunVmLock(vmId).getExclusiveLocks().
The drawback is that it is different from the regular way we create locks.
I think the bigger problem is that there's no place where we can get the big
picture of the locks, let's say that we want to see all the commands that take
VM locks, we need to find all its usages.. and this it is more complicated to
ensure that changing the locks in one place doesn't affect other places that
way..
What do you think? if you still want just to extract it somewhere, where do you
think it should be?
Line 56: return new EngineLock(
Line 57: Collections.singletonMap(
Line 58: vmId.toString(),
Line 59: LockMessagesMatchUtil.makeLockingPair(
--
To view, visit http://gerrit.ovirt.org/19499
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0968cae554458f53a1f30643848bb8d88bd4de11
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
