Shireesh Anjal has posted comments on this change. Change subject: engine: Refresh gluster data periodically ......................................................................
Patch Set 2: (5 inline comments) Responses to comments in-line. New patch-set to follow. .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java Line 99: private boolean glusterModeSupported() { Line 100: VdcQueryReturnValue result = Line 101: Backend.getInstance().RunPublicQuery(VdcQueryType.GetConfigurationValue, Line 102: new GetConfigurationValueParameters(ConfigurationValues.ApplicationMode, Line 103: Config.DefaultConfigurationVersion)); Done Line 104: if (result.getSucceeded()) { Line 105: Integer appMode = (Integer)result.getReturnValue(); Line 106: return ((appMode & ApplicationMode.GlusterOnly.getValue()) > 0); Line 107: } else { Line 136: Line 137: private void refreshClusterData(VDSGroup cluster) { Line 138: VDS upServer = getClusterUtils().getUpServer(cluster.getId()); Line 139: if (upServer == null) { Line 140: log.warnFormat("No server UP in cluster {0}. Can't refresh it's data at this point.", cluster.getname()); Done Line 141: return; Line 142: } Line 143: Line 144: refreshServerData(cluster, upServer); Line 154: * @param cluster Line 155: * @param upServer Line 156: */ Line 157: private void refreshServerData(VDSGroup cluster, VDS upServer) { Line 158: if(cluster.supportsVirtService()) { Done Line 159: // If the cluster supports virt service as well, we should not be removing any servers from it, even if they Line 160: // have been removed from the Gluster cluster using the Gluster cli, as they could potentially be used for Line 161: // running VMs Line 162: return; Line 204: private Set<GlusterServerInfo> fetchServers(VDSGroup cluster, VDS upServer, List<VDS> existingServers) { Line 205: Set<GlusterServerInfo> fetchedServers; Line 206: Line 207: GlusterServersListVDSCommand<VdsIdVDSCommandParametersBase> serversListCommand = Line 208: createServersListCommand(upServer); Done Line 209: Line 210: fetchedServers = (Set<GlusterServerInfo>)serversListCommand.ExecuteWithReturnValue(); Line 211: if(fetchedServers.size() == 1 && existingServers.size() > 2) { Line 212: // It's possible that the server we are using to get list of servers itself has been removed from the Line 207: GlusterServersListVDSCommand<VdsIdVDSCommandParametersBase> serversListCommand = Line 208: createServersListCommand(upServer); Line 209: Line 210: fetchedServers = (Set<GlusterServerInfo>)serversListCommand.ExecuteWithReturnValue(); Line 211: if(fetchedServers.size() == 1 && existingServers.size() > 2) { The idea behind this code is as follows: There is a possibility that one of the servers of the cluster was removed from gluster cluster using CLI, and we coincidentally picked the same one as "UP" server. In this case, the server list returned by the VDS command will show just that server. If we assume that this is the correct list of servers, we might end up deleting all other (n-1) servers from the engine, which is not good. So if we encounter such a case, we fetch a different UP server, which is guaranteed to be from the other side (with n-1 servers), thus giving us the "good" set of servers. If existing servers were just 2, then it really doesn't matter which one we keep in the engine DB and which one we remove - so no issues. Line 212: // It's possible that the server we are using to get list of servers itself has been removed from the Line 213: // cluster, and hence is returning a single server (itself) Line 214: GlusterServerInfo server = fetchedServers.iterator().next(); Line 215: if(server.getHostnameOrIp().equals(upServer.gethost_name()) || server.getHostnameOrIp().equals(upServer.getManagmentIp())) { -- To view, visit http://gerrit.ovirt.org/7288 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b61eb6e93105e46e2706eac1d94bc10717224c2 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches