Sahina Bose has posted comments on this change. Change subject: engine: Peer probing with alternate addresses ......................................................................
Patch Set 11: (8 comments) https://gerrit.ovirt.org/#/c/38149/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java: Line 216: } Line 217: } Line 218: Line 219: private void peerProbeAlternateInterfaces(Network glusterNetwork, VDS host) { Line 220: GlusterServer glusterServer = getGlusterServerDao().get(host.getId()); > you can split this condition, so in case glusterNetwork is null or the stat Done Line 221: if (glusterNetwork != null && host.getStatus() == VDSStatus.Up Line 222: && glusterServer != null) { Line 223: List<VdsNetworkInterface> interfaces = getInterfaceDao().getAllInterfacesForVds(host.getId()); Line 224: for (VdsNetworkInterface iface : interfaces) { Line 222: && glusterServer != null) { Line 223: List<VdsNetworkInterface> interfaces = getInterfaceDao().getAllInterfacesForVds(host.getId()); Line 224: for (VdsNetworkInterface iface : interfaces) { Line 225: if (glusterNetwork.getName().equals(iface.getNetworkName()) && Line 226: !StringUtils.isBlank(iface.getAddress()) > could be replaced by StringUtils.isNotBlank() instead of negating the isBla Done Line 227: && !glusterServer.getKnownAddresses().contains(iface.getAddress())) { Line 228: // get another server in the cluster Line 229: VDS upServer = getAlternateUpServerInCluster(host.getVdsGroupId(), Line 230: host.getId()); Line 225: if (glusterNetwork.getName().equals(iface.getNetworkName()) && Line 226: !StringUtils.isBlank(iface.getAddress()) Line 227: && !glusterServer.getKnownAddresses().contains(iface.getAddress())) { Line 228: // get another server in the cluster Line 229: VDS upServer = getAlternateUpServerInCluster(host.getVdsGroupId(), > the two lines could be merged by the formatter (they aren't exceed 120 char Done Line 230: host.getId()); Line 231: if (upServer != null) { Line 232: Boolean peerProbed = glusterPeerProbeAdditionalInterface(upServer.getId(), iface.getAddress()); Line 233: if (peerProbed) { Line 228: // get another server in the cluster Line 229: VDS upServer = getAlternateUpServerInCluster(host.getVdsGroupId(), Line 230: host.getId()); Line 231: if (upServer != null) { Line 232: Boolean peerProbed = glusterPeerProbeAdditionalInterface(upServer.getId(), iface.getAddress()); > should be primitive boolean Done Line 233: if (peerProbed) { Line 234: getGlusterServerDao().addKnownAddress(host.getId(), iface.getAddress()); Line 235: } Line 236: } Line 252: Line 253: private VDS getAlternateUpServerInCluster(Guid clusterId, Guid vdsId) { Line 254: List<VDS> vdsList = getVdsDao().getAllForVdsGroupWithStatus(clusterId, VDSStatus.Up); Line 255: // If the cluster already having Gluster servers, get an up server Line 256: if (vdsList != null && vdsList.size() > 0) { > vdsList cannot be null by the contract of the dao for fetching collections. Done Line 257: for (VDS vds : vdsList) { Line 258: if (!vdsId.equals(vds.getId())) { Line 259: return vds; Line 260: } https://gerrit.ovirt.org/#/c/38149/11/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterServer.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterServer.java: Line 11: private static final long serialVersionUID = -1425566208615075937L; Line 12: Line 13: private Guid serverId; Line 14: Line 15: private ArrayList<String> knownAddresses; > can't it be just List ? I think GWT requires concrete type, IIRC Line 16: Line 17: private Guid glusterServerUuid; Line 18: Line 19: public GlusterServer() { https://gerrit.ovirt.org/#/c/38149/11/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterServerDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterServerDaoDbFacadeImpl.java: Line 48: GlusterServer glusterServer = new GlusterServer(); Line 49: glusterServer.setId(getGuidDefaultEmpty(rs, "server_id")); Line 50: glusterServer.setGlusterServerUuid(getGuidDefaultEmpty(rs, "gluster_server_uuid")); Line 51: String knownAddresses = rs.getString("known_addresses"); Line 52: if (!StringUtils.isBlank(knownAddresses)) { > use StringUtils.isNotBlank() Done Line 53: String[] knownAddressArray = knownAddresses.split(","); Line 54: ArrayList<String> knownAddressList = new ArrayList<>(); Line 55: for (String addr : knownAddressArray) { Line 56: knownAddressList.add(addr); Line 50: glusterServer.setGlusterServerUuid(getGuidDefaultEmpty(rs, "gluster_server_uuid")); Line 51: String knownAddresses = rs.getString("known_addresses"); Line 52: if (!StringUtils.isBlank(knownAddresses)) { Line 53: String[] knownAddressArray = knownAddresses.split(","); Line 54: ArrayList<String> knownAddressList = new ArrayList<>(); > can type be reduced to List ? How do you mean? Change it in the POJO? Line 55: for (String addr : knownAddressArray) { Line 56: knownAddressList.add(addr); Line 57: } Line 58: glusterServer.setKnownAddresses(knownAddressList); -- To view, visit https://gerrit.ovirt.org/38149 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8fa407d6a525e73b89a79d063517798283b520fd Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sahina Bose <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
