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

Reply via email to