Michael Kublin has uploaded a new change for review.
Change subject: engine: Replacing getVds() query by getVdsStatic() query in
VDSCommands
......................................................................
engine: Replacing getVds() query by getVdsStatic() query in VDSCommands
The following patch will replace a getVds() query by getVdsStatic() query in
almost all vds (not spm) commands.
THe reasons are:
1. A getVds() query - is a query to view, which is based on JOIN from at least
7 tables, getVdsStatic() it is a query
to single table, meaning much more faster.
2. VDSStatic object is smaller than VDS object, less resources spent on
retrievening it from DB, allocatin memory, etc...
3. VDSStatic object almost never changed, it is means that when it will be
cached, we will not perfrom any query during
vds commands, the VDS object contains VDSDynamic part, which is changed very
frequently, so caching of VDS object is more
difficult and possible not efficient.
4. No reason to use VDS object , when we can use VDSStatic object
Also patch includes a following fixes and improvements:
1. FullListVDSCommand will never perform a query to vds object, it will be
passed via parameters
2. A part of code moved from GetCapabilitiesVDSCommand to
CollectVdsNetworkDataVDSCommand, this code is not relevant
for GetCapabilitiesVDSCommand and only relevant for
CollectVdsNetworkDataVDSCommand
3. Code clean up
Also, after applying a patch, a query for VDSStatic will be made only if it is
required to log command, and not always in
constractor
Change-Id: Iff6fa35e25bcd895c25359ece6ba4d5ce9c8e8cf
Signed-off-by: Michael Kublin <mkub...@redhat.com>
---
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/FullListVDSCommandParameters.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CollectVdsNetworkDataVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateVGVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetCapabilitiesVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetVmTicketVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStartVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStatusVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStopVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmStatsVdsBrokerCommand.java
M
backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java
12 files changed, 45 insertions(+), 40 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/21/14321/1
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/FullListVDSCommandParameters.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/FullListVDSCommandParameters.java
index 14252f9..3a0bbf0 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/FullListVDSCommandParameters.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/FullListVDSCommandParameters.java
@@ -2,7 +2,7 @@
import java.util.List;
-import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.common.businessentities.VDS;
/**
* This class is for the list verb that supports getting "full" VM data for a
given list of VMs
@@ -11,11 +11,8 @@
private List<String> vmIds;
- public FullListVDSCommandParameters() {
- }
-
- public FullListVDSCommandParameters(Guid vdsId, List<String> vmIds) {
- super(vdsId);
+ public FullListVDSCommandParameters(VDS vds, List<String> vmIds) {
+ super(vds);
this.vmIds = vmIds;
}
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
index f874db5..30e9b38 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
@@ -1098,7 +1098,7 @@
*/
protected Map[] getVmInfo(List<String> vmsToUpdate) {
return (Map[]) (new FullListVdsCommand<FullListVDSCommandParameters>(
- new FullListVDSCommandParameters(_vds.getId(),
vmsToUpdate)).executeWithReturnValue());
+ new FullListVDSCommandParameters(_vds,
vmsToUpdate)).executeWithReturnValue());
}
private boolean shouldLogDeviceDetails(String deviceType) {
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CollectVdsNetworkDataVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CollectVdsNetworkDataVDSCommand.java
index cc3de76..e417270 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CollectVdsNetworkDataVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CollectVdsNetworkDataVDSCommand.java
@@ -34,6 +34,10 @@
extends GetCapabilitiesVDSCommand<P> {
public CollectVdsNetworkDataVDSCommand(P parameters) {
super(parameters);
+ if (getVds() == null) {
+
setVdsAndVdsStatic(DbFacade.getInstance().getVdsDao().get(parameters.getVdsId()));
+ parameters.setVds(getVds());
+ }
}
@Override
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateVGVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateVGVDSCommand.java
index 28a4132..9cb7093 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateVGVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateVGVDSCommand.java
@@ -6,12 +6,13 @@
import org.ovirt.engine.core.common.config.Config;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.vdscommands.*;
+import org.ovirt.engine.core.dal.dbbroker.DbFacade;
public class CreateVGVDSCommand<P extends CreateVGVDSCommandParameters>
extends VdsBrokerCommand<P> {
private OneUuidReturnForXmlRpc _result;
public CreateVGVDSCommand(P parameters) {
- super(parameters);
+ super(parameters,
DbFacade.getInstance().getVdsDao().get(parameters.getVdsId()));
}
@Override
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetCapabilitiesVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetCapabilitiesVDSCommand.java
index bbbd9a8..ec06eb9 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetCapabilitiesVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetCapabilitiesVDSCommand.java
@@ -1,7 +1,6 @@
package org.ovirt.engine.core.vdsbroker.vdsbroker;
import
org.ovirt.engine.core.common.vdscommands.VdsIdAndVdsVDSCommandParametersBase;
-import org.ovirt.engine.core.dal.dbbroker.DbFacade;
import org.ovirt.engine.core.utils.log.Logged;
import org.ovirt.engine.core.utils.log.Logged.LogLevel;
@@ -9,10 +8,6 @@
public class GetCapabilitiesVDSCommand<P extends
VdsIdAndVdsVDSCommandParametersBase> extends InfoVdsBrokerCommand<P> {
public GetCapabilitiesVDSCommand(P parameters) {
super(parameters, parameters.getVds());
- if (getVds() == null) {
-
setVds(DbFacade.getInstance().getVdsDao().get(parameters.getVdsId()));
- parameters.setVds(getVds());
- }
}
@Override
@@ -20,6 +15,5 @@
infoReturn = getBroker().getCapabilities();
ProceedProxyReturnValue();
VdsBrokerObjectsBuilder.updateVDSDynamicData(getVds(),
infoReturn.mInfo);
-
}
}
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetVmTicketVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetVmTicketVDSCommand.java
index c71bef8..983e379 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetVmTicketVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetVmTicketVDSCommand.java
@@ -7,6 +7,7 @@
import org.ovirt.engine.core.common.vdscommands.*;
import org.ovirt.engine.core.common.config.Config;
import org.ovirt.engine.core.common.config.ConfigValues;
+import org.ovirt.engine.core.dal.dbbroker.DbFacade;
public class SetVmTicketVDSCommand<P extends SetVmTicketVDSCommandParameters>
extends VdsBrokerCommand<P> {
private Guid mVmId = new Guid();
@@ -15,7 +16,7 @@
private String connectionAction = "disconnect";
public SetVmTicketVDSCommand(P parameters) {
- super(parameters);
+ super(parameters,
DbFacade.getInstance().getVdsDao().get(parameters.getVdsId()));
mVmId = parameters.getVmId();
mTicket = parameters.getTicket();
mValidTime = parameters.getValidTime();
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStartVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStartVDSCommand.java
index f986749..df1f276 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStartVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStartVDSCommand.java
@@ -17,9 +17,11 @@
public class SpmStartVDSCommand<P extends SpmStartVDSCommandParameters>
extends VdsBrokerCommand<P> {
public SpmStartVDSCommand(P parameters) {
super(parameters);
+ vdsId = parameters.getVdsId();
}
private OneUuidReturnForXmlRpc _result;
+ private Guid vdsId;
@Override
protected void ExecuteVdsBrokerCommand() {
@@ -39,7 +41,7 @@
taskStatus = (AsyncTaskStatus) ResourceManager
.getInstance()
.runVdsCommand(VDSCommandType.HSMGetTaskStatus,
- new
HSMTaskGuidBaseVDSCommandParameters(getVds().getId(), taskId)).getReturnValue();
+ new HSMTaskGuidBaseVDSCommandParameters(vdsId,
taskId)).getReturnValue();
log.debugFormat("spmStart polling - task status: {0}",
taskStatus.getStatus().toString());
} while (taskStatus.getStatus() != AsyncTaskStatusEnum.finished
&& taskStatus.getStatus() != AsyncTaskStatusEnum.unknown);
@@ -53,7 +55,7 @@
SpmStatusResult spmStatus = (SpmStatusResult) ResourceManager
.getInstance()
.runVdsCommand(VDSCommandType.SpmStatus,
- new SpmStatusVDSCommandParameters(getVds().getId(),
getParameters().getStoragePoolId()))
+ new SpmStatusVDSCommandParameters(vdsId,
getParameters().getStoragePoolId()))
.getReturnValue();
if (spmStatus != null) {
log.infoFormat("spmStart polling ended, spm status: {0}",
spmStatus.getSpmStatus().toString());
@@ -62,7 +64,7 @@
}
try {
ResourceManager.getInstance().runVdsCommand(VDSCommandType.HSMClearTask,
- new HSMTaskGuidBaseVDSCommandParameters(getVds().getId(),
taskId));
+ new HSMTaskGuidBaseVDSCommandParameters(vdsId, taskId));
} catch (java.lang.Exception e) {
log.errorFormat("Could not clear spmStart task (id - {0}),
continuing with SPM selection.", taskId);
}
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStatusVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStatusVDSCommand.java
index 5bea7df..9cbd9f8 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStatusVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStatusVDSCommand.java
@@ -16,7 +16,7 @@
private SpmStatusReturnForXmlRpc _result;
public SpmStatusVDSCommand(P parameters) {
- super(parameters, null);
+ super(parameters);
}
@Override
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStopVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStopVDSCommand.java
index 6e2a38b..fac603d 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStopVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStopVDSCommand.java
@@ -9,11 +9,12 @@
import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
import org.ovirt.engine.core.common.vdscommands.VdsIdVDSCommandParametersBase;
import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.dal.dbbroker.DbFacade;
import org.ovirt.engine.core.vdsbroker.ResourceManager;
public class SpmStopVDSCommand<P extends SpmStopVDSCommandParameters> extends
VdsBrokerCommand<P> {
public SpmStopVDSCommand(P parameters) {
- super(parameters);
+ super(parameters,
DbFacade.getInstance().getVdsDao().get(parameters.getVdsId()));
}
@Override
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerCommand.java
index cb564ac..128e36f 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerCommand.java
@@ -2,6 +2,7 @@
import org.apache.commons.lang.exception.ExceptionUtils;
import org.ovirt.engine.core.common.businessentities.VDS;
+import org.ovirt.engine.core.common.businessentities.VdsStatic;
import org.ovirt.engine.core.common.errors.VDSError;
import org.ovirt.engine.core.common.errors.VdcBLLException;
import org.ovirt.engine.core.common.errors.VdcBllErrors;
@@ -14,7 +15,8 @@
public abstract class VdsBrokerCommand<P extends
VdsIdVDSCommandParametersBase> extends BrokerCommandBase<P> {
private final IVdsServer mVdsBroker;
- private VDS mVds;
+ private VdsStatic vdsStatic;
+ private VDS vds;
/**
* Construct the command using the parameters and the {@link VDS} which is
loaded from the DB.
*
@@ -24,7 +26,6 @@
public VdsBrokerCommand(P parameters) {
super(parameters);
mVdsBroker = initializeVdsBroker(parameters.getVdsId());
- mVds = getDbFacade().getVdsDao().get(parameters.getVdsId());
}
/**
@@ -37,8 +38,9 @@
*/
protected VdsBrokerCommand(P parameters, VDS vds) {
super(parameters);
- mVdsBroker = initializeVdsBroker(parameters.getVdsId());
- mVds = vds;
+ this.mVdsBroker = initializeVdsBroker(parameters.getVdsId());
+ this.vds = vds;
+ this.vdsStatic = vds.getStaticData();
}
protected IVdsServer initializeVdsBroker(Guid vdsId) {
@@ -61,19 +63,27 @@
@Override
protected String getAdditionalInformation() {
- if (getVds() != null) {
- return String.format("HostName = %1$s", getVds().getName());
+ if (getAndSetVdsStatic() != null) {
+ return String.format("HostName = %1$s",
getAndSetVdsStatic().getName());
} else {
return super.getAdditionalInformation();
}
}
- protected VDS getVds() {
- return mVds;
+ private VdsStatic getAndSetVdsStatic() {
+ if (vdsStatic == null) {
+ vdsStatic =
getDbFacade().getVdsStaticDao().get(getParameters().getVdsId());
+ }
+ return vdsStatic;
}
- protected void setVds(VDS value) {
- mVds = value;
+ protected VDS getVds() {
+ return vds;
+ }
+
+ protected void setVdsAndVdsStatic(VDS vds) {
+ this.vds = vds;
+ this.vdsStatic = vds.getStaticData();
}
protected DbFacade getDbFacade() {
@@ -98,12 +108,12 @@
// TODO: look for invalid certificates error handling
catch (RuntimeException e) {
PrintReturnValue();
- if (getVds() == null) {
+ if (getAndSetVdsStatic() == null) {
log.errorFormat("Failed in {0} method, for vds id: {1}",
getCommandName(), getParameters().getVdsId());
} else {
log.errorFormat("Failed in {0} method, for vds: {1}; host:
{2}",
- getCommandName(), getVds().getName(),
getVds().getHostName());
+ getCommandName(), getAndSetVdsStatic().getName(),
getAndSetVdsStatic().getHostName());
}
throw e;
}
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmStatsVdsBrokerCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmStatsVdsBrokerCommand.java
index 3c49d2b..172c14a 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmStatsVdsBrokerCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmStatsVdsBrokerCommand.java
@@ -10,14 +10,9 @@
public abstract class VmStatsVdsBrokerCommand<P extends
VdsIdVDSCommandParametersBase> extends VdsBrokerCommand<P> {
protected VMInfoListReturnForXmlRpc mVmListReturn;
- public VmStatsVdsBrokerCommand(P parameters) {
- super(parameters);
- }
-
protected VmStatsVdsBrokerCommand(P parameters, VDS vds) {
super(parameters, vds);
}
-
@Override
protected StatusForXmlRpc getReturnStatus() {
diff --git
a/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java
b/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java
index fe2b177..317a902 100644
---
a/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java
+++
b/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java
@@ -27,7 +27,7 @@
import
org.ovirt.engine.core.common.vdscommands.SetupNetworksVdsCommandParameters;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.dbbroker.DbFacade;
-import org.ovirt.engine.core.dao.VdsDAO;
+import org.ovirt.engine.core.dao.VdsStaticDAO;
import org.ovirt.engine.core.utils.RandomUtils;
@RunWith(MockitoJUnitRunner.class)
@@ -192,9 +192,9 @@
private SetupNetworksVDSCommand<SetupNetworksVdsCommandParameters>
createCommand(
SetupNetworksVdsCommandParameters parameters) {
final DbFacade dbFacade = mock(DbFacade.class);
- final VdsDAO vdsDao = mock(VdsDAO.class);
+ final VdsStaticDAO vdsStaticDao = mock(VdsStaticDAO.class);
- when(dbFacade.getVdsDao()).thenReturn(vdsDao);
+ when(dbFacade.getVdsStaticDao()).thenReturn(vdsStaticDao);
// No way to avoid these calls by regular mocking, so must implement
anonymously.
return new
SetupNetworksVDSCommand<SetupNetworksVdsCommandParameters>(parameters) {
--
To view, visit http://gerrit.ovirt.org/14321
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iff6fa35e25bcd895c25359ece6ba4d5ce9c8e8cf
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <mkub...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
