Sahina Bose has posted comments on this change.

Change subject: engine : Query to fetch list of volumes eligible for geo 
replication
......................................................................


Patch Set 5:

(3 comments)

Will need to add other criteria -
1)  that the volume is not used already as a destination volume. 
2) the volume's capacity is greater than equal to master volume's capacity
3) the volume's usage is 0

http://gerrit.ovirt.org/#/c/33845/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetEligibleVolumesForGeoRepEnablingQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetEligibleVolumesForGeoRepEnablingQuery.java:

Line 19:     @Override
Line 20:     protected void executeQueryCommand() {
Line 21:         List<GlusterVolumeEntity> eligibleVolumeList = new 
ArrayList<GlusterVolumeEntity>();
Line 22:         Guid masterVolumeId = getParameters().getVolumeId();
Line 23:         DbFacade dbFacadeInstance = getDbFacade();
why do you need to declare this variable? Why not use 
getDbFacade().getVdsGroupDao below
Line 24:         List<VDSGroup> vdsGroups = 
dbFacadeInstance.getVdsGroupDao().getAll();
Line 25:         for (VDSGroup vdsGroup : vdsGroups) {
Line 26:             if (vdsGroup.getId() != 
getGlusterVolumeDao().getById(masterVolumeId).getClusterId()) {
Line 27:                 List<GlusterVolumeEntity> volumes = 
getGlusterVolumeDao().getByClusterId(vdsGroup.getId());


Line 22:         Guid masterVolumeId = getParameters().getVolumeId();
Line 23:         DbFacade dbFacadeInstance = getDbFacade();
Line 24:         List<VDSGroup> vdsGroups = 
dbFacadeInstance.getVdsGroupDao().getAll();
Line 25:         for (VDSGroup vdsGroup : vdsGroups) {
Line 26:             if (vdsGroup.getId() != 
getGlusterVolumeDao().getById(masterVolumeId).getClusterId()) {
You can avoid a db call for every vdsgroup, by getting the mastervolume entity 
outside of the loop.
Line 27:                 List<GlusterVolumeEntity> volumes = 
getGlusterVolumeDao().getByClusterId(vdsGroup.getId());
Line 28:                 for (GlusterVolumeEntity volume : volumes) {
Line 29:                     if (volume.getId() != masterVolumeId && 
volume.getStatus() == GlusterStatus.UP) {
Line 30:                         eligibleVolumeList.add(volume);


Line 25:         for (VDSGroup vdsGroup : vdsGroups) {
Line 26:             if (vdsGroup.getId() != 
getGlusterVolumeDao().getById(masterVolumeId).getClusterId()) {
Line 27:                 List<GlusterVolumeEntity> volumes = 
getGlusterVolumeDao().getByClusterId(vdsGroup.getId());
Line 28:                 for (GlusterVolumeEntity volume : volumes) {
Line 29:                     if (volume.getId() != masterVolumeId && 
volume.getStatus() == GlusterStatus.UP) {
volume.getId() would never be equal to masterVolumeId as you are retrieving 
volumes outside of the master volume's cluster, right?
Line 30:                         eligibleVolumeList.add(volume);
Line 31:                     }
Line 32:                 }
Line 33:             }


-- 
To view, visit http://gerrit.ovirt.org/33845
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc3ecb15535181f1ca2a8780461cb89788a3f41
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Ramesh N <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: anmolbabu <[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

Reply via email to