Liron Aravot has posted comments on this change.
Change subject: core: Remove global lock on SPM calls from engine side
......................................................................
Patch Set 5: (3 inline comments)
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
Line 75:
Line 76: @Logged(errorLevel = LogLevel.ERROR)
Line 77: public abstract class IrsBrokerCommand<P extends
IrsBaseVDSCommandParameters> extends BrokerCommandBase<P> {
Line 78:
Line 79: private Guid currentCommandSpmId;
two questions regarding this change
1. can you change the name from mTriedVdssList? i know it's a copy-paste, but
that's a set with "List" in it's name and it's now added to many more places in
the code..worth changing in my opinion.
2. can you elaborate why did you move it here?
Line 80: protected final Set<Guid> mTriedVdssList = new HashSet<Guid>();
Line 81:
Line 82: public static final long BYTES_TO_GB = 1024 * 1024 * 1024;
Line 83: public static final int DAY_IN_MILLIS = 24 * 60 * 60 * 1000;
Line 440: setmIrsPort(vds.getport());
Line 441: privatemCurrentIrsHost = vds.gethost_name();
Line 442: }
Line 443:
Line 444: public boolean failover(Guid currentCommandSpmId, Set<Guid>
mTriedVdssList) {
seems like there's a new corner case now, correct me if i'm wrong - if a
command A currentCommandSpmId is succesfully started as SPM by another command
failover, command A will still perform spm stop though everything is now fine.
can we somehow eliminate this? maybe having the time spm was chosen and not
perform anything if it's later then when we started waiting?
Line 445: synchronized (syncObj) {
Line 446: mTriedVdssList.add(currentCommandSpmId);
Line 447: if (currentCommandSpmId.equals(mCurrentVdsId)) {
Line 448: if (getHasVdssForSpmSelection(mCurrentVdsId,
mTriedVdssList)) {
Line 441: privatemCurrentIrsHost = vds.gethost_name();
Line 442: }
Line 443:
Line 444: public boolean failover(Guid currentCommandSpmId, Set<Guid>
mTriedVdssList) {
Line 445: synchronized (syncObj) {
seems like this add should be after the if , no?
Line 446: mTriedVdssList.add(currentCommandSpmId);
Line 447: if (currentCommandSpmId.equals(mCurrentVdsId)) {
Line 448: if (getHasVdssForSpmSelection(mCurrentVdsId,
mTriedVdssList)) {
Line 449: Guid vdsId = mCurrentVdsId;
--
To view, visit http://gerrit.ovirt.org/9298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9779fa4b5a4da2ef3ca1d62fe5203a3949ce278
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Daniel Paikov <pai...@gmail.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
