Eli Mesika has posted comments on this change. Change subject: [RFE] Improve fencing robustness by retrying... ......................................................................
Patch Set 1: (4 comments) http://gerrit.ovirt.org/#/c/27309/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceExecutor.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceExecutor.java: Line 137: } Line 138: return !NO_VDS.equals(proxyHostId); Line 139: } Line 140: Line 141: private synchronized boolean findProxyHost(Guid id) { > +1 on 2nd suggestion. of course, change the parameter name as well. Done Line 142: skippedProxyHostId = id; Line 143: boolean res = findProxyHost(); Line 144: skippedProxyHostId=null; Line 145: return res; Line 137: } Line 138: return !NO_VDS.equals(proxyHostId); Line 139: } Line 140: Line 141: private synchronized boolean findProxyHost(Guid id) { > I would prefer better name, it's the same as in findProxyHost() but the fun Done Line 142: skippedProxyHostId = id; Line 143: boolean res = findProxyHost(); Line 144: skippedProxyHostId=null; Line 145: return res; Line 304: private VDS getFenceProxy(final boolean onlyUpHost, final boolean filterSelf, final PMProxyOptions proxyOptions) { Line 305: List<VDS> hosts = DbFacade.getInstance().getVdsDao().getAll(); Line 306: // If a skippedProxyHostId was given, try to use another proxy Line 307: if (skippedProxyHostId != null) { Line 308: for (int i=0; i < hosts.size(); i++) { > Agreed. otherwise ConcurrentModificationException may be thrown. Done Line 309: if (hosts.get(i).getId().equals(skippedProxyHostId)) { Line 310: hosts.remove(i); Line 311: break; Line 312: } Line 304: private VDS getFenceProxy(final boolean onlyUpHost, final boolean filterSelf, final PMProxyOptions proxyOptions) { Line 305: List<VDS> hosts = DbFacade.getInstance().getVdsDao().getAll(); Line 306: // If a skippedProxyHostId was given, try to use another proxy Line 307: if (skippedProxyHostId != null) { Line 308: for (int i=0; i < hosts.size(); i++) { > I'd prefer to use iterator for traversing collection and iterator.remove() Done Line 309: if (hosts.get(i).getId().equals(skippedProxyHostId)) { Line 310: hosts.remove(i); Line 311: break; Line 312: } -- To view, visit http://gerrit.ovirt.org/27309 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I817e8a03695d02ee3f59fbefe0fe41483c3989fe Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
