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

Reply via email to