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 <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches