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 <yzasl...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liran Zelkha <liran.zel...@gmail.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches