Michael Kublin 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;
First of all Liron, about code reviews, when you have question or remarks you
should ask them at the first revision and not in the third, these should be
done in order not waste my time and yours.
Second, I am not changing a name and will not send a new patch because of
these, especially when I am already at fourth revision. Read previous remark.
By the way it is not copy paste, these was old name and it left.
Third, in order to keep it per thread with out unneeded synchronization
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) {
Liron, you are wrong. Lets think how these can be: I failed at SPM, I came to
fail over, some other thread failed with my spm also and choose another one, so
a third one failed with another one and choose my and success and now my thread
perform fail over for it? What is a probability for that case? Don't see a
reason to handle it.
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) {
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
