Allon Mureinik has posted comments on this change.
Change subject: core: Introducing Batch updates to DAOs
......................................................................
Patch Set 9: I would prefer that you didn't submit this
(7 inline comments)
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbEngineDialect.java
Line 53: * @param procNameFromDB
Line 54: * @param params
Line 55: */
Line 56: public String createSqlCallCommand(String procSchemaFromDB,
Line 57: String procNameFromDB, StringBuffer params);
why can't the params be a String?
Line 58:
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/PostgresDbEngineDialect.java
Line 198:
Line 199: @Override
Line 200: public String createSqlCallCommand(String procSchemaFromDB,
Line 201: String procNameFromDB, StringBuffer params) {
Line 202: StringBuffer sqlCommand = new StringBuffer();
why not a faster StringBuilder?
Line 203: sqlCommand.append("{call
").append(procSchemaFromDB).append(".")
Line 204:
.append(procNameFromDB).append("(").append(params).append(")}");
Line 205: return sqlCommand.toString();
Line 206: }
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/SimpleJdbcCallsHandler.java
Line 23: import org.springframework.jdbc.core.simple.SimpleJdbcCall;
Line 24:
Line 25: public class SimpleJdbcCallsHandler {
Line 26: private static final Log log = LogFactory
Line 27: .getLog(SimpleJdbcCallsHandler.class);
The \n here should probably be removed
Line 28:
Line 29: private ConcurrentMap<String, SimpleJdbcCall> callsMap =
Line 30: new ConcurrentHashMap<String, SimpleJdbcCall>();
Line 31:
Line 29: private ConcurrentMap<String, SimpleJdbcCall> callsMap =
Line 30: new ConcurrentHashMap<String, SimpleJdbcCall>();
Line 31:
Line 32: private ConcurrentMap<String, StoredProcedureMetaData>
storedProceduresMap =
Line 33: new ConcurrentHashMap<String, StoredProcedureMetaData>();
Consider using Java 7's syntax: new ConcurrentHashMap<>
Line 34:
Line 35: private DbEngineDialect dialect;
Line 36:
Line 37: private JdbcTemplate template;
Line 77: procMetaData.getParamatersMetaData());
Line 78: stmt.addBatch();
Line 79: }
Line 80:
Line 81: try {
The try should encompass all the work with the statement, not just the
executable.
Also, why not use Java7 autoclosable syntax?
Line 82: stmt.executeBatch();
Line 83: log.debug("Executed batch");
Line 84: } catch (SQLException e) {
Line 85: log.fatal("Can't execute batch: ", e);
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java
Line 22: import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
Line 23:
Line 24: public class InterfaceDaoDbFacadeImpl extends BaseDAODbFacade
implements InterfaceDao {
Line 25: private static final Log log = LogFactory
Line 26: .getLog(InterfaceDaoDbFacadeImpl.class);
The \n here should probably be removed
Line 27:
Line 28: @Override
Line 29: public void saveStatisticsForVds(VdsNetworkStatistics stats) {
Line 30: MapSqlParameterSource parameterSource =
getCustomMapSqlParameterSource()
Line 70:
Line 71: @Override
Line 72: public void
massUpdateStatisticsForVds(Collection<VdsNetworkStatistics> statistics) {
Line 73: try {
Line 74: List<MapSqlParameterSource> executions = new
ArrayList<MapSqlParameterSource>();
Initialize it with statistics.size()
Line 75: for (VdsNetworkStatistics stats : statistics) {
Line 76: executions.add(getCustomMapSqlParameterSource()
Line 77: .addValue("id", stats.getId())
Line 78: .addValue("rx_drop",
stats.getReceiveDropRate())
--
To view, visit http://gerrit.ovirt.org/15039
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If5ee0aa90bca3b5c257beb7b0eaa236f02f0206f
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
