Yair Zaslavsky has posted comments on this change.

Change subject: [WIP]core : BLL changes for Multi-Tier fencing
......................................................................


Patch Set 3: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceVdsBaseCommand.java
Line 240:      * for Start  we should have two concurrent threads and wait for 
one to succeed
Line 241:      * @param lastStatus
Line 242:      */
Line 243:     private void handleMultipleConcurrentAgents(VDSStatus lastStatus, 
VDSReturnValue vdsReturnValue) {
Line 244:         final CountDownLatch latch = (getParameters().getAction() == 
FenceActionType.Stop) ?
I thought why you needed nested trinary if operator, and I understand (due to 
the technicalities of the "final" keyword).
Please consider separating this piece of code to another method - and inside it 
you will be able to uses non final CountdownLatch , have switch and not nested 
trinary if, and simply return the created latch and set it at "final 
CountdownLatch"
Line 245:                 new CountDownLatch(2) // both agents should succeed 
in Stop
Line 246:                         : (getParameters().getAction() == 
FenceActionType.Start)
Line 247:                             ?
Line 248:                             new CountDownLatch(1) //only one agent 
should succeed in Start


Line 264:             });
Line 265:             try {
Line 266:                 latch.await();
Line 267:             } catch (InterruptedException e) {
Line 268:                 log.error(e);
Although most likely it will not happen, please don't log the error this way.
I would suggest you perform logging of a more meaningful message with error 
level, and at debug level log the stack trace:
log.error("An error occurred while waiting for multiple agents to perform a 
fence operation)  //you can also specify which fence operation
and then on the next line -
log.debug(e);
Line 269:             }
Line 270:             switch (getParameters().getAction()) {
Line 271:             case Start :
Line 272:                 if (primarySucceeded || secondarySucceeded){


....................................................
Commit Message
Line 5: CommitDate: 2012-12-23 01:55:39 +0200
Line 6: 
Line 7: [WIP]core : BLL changes for Multi-Tier fencing
Line 8: 
Line 9: this patch includes masically changes in the fencing infrastructures and
What did you try to write here?:)
Line 10: supported queries.
Line 11: 
Line 12: Change-Id: I82f243593f2b361ca75d97e06f9aede246d4a1b1


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82f243593f2b361ca75d97e06f9aede246d4a1b1
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@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

Reply via email to