Mike Kolesnik has posted comments on this change. Change subject: core: Introducing Batch updates to DAOs ......................................................................
Patch Set 9: (4 inline comments) r .................................................... 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 use StringBuilder? You're always creating a new instance, and it's not used by multiple threads. 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 59: public void executeStoredProcAsBatch(final String procName, Line 60: final List<MapSqlParameterSource> executions) Line 61: throws DataAccessException { Line 62: Line 63: template.execute(new ConnectionCallback<Object>() { Is it possible to extract this to an inner class? This makes the methods way too bloated.. Line 64: Line 65: @Override Line 66: public Object doInConnection(Connection con) throws SQLException, Line 67: DataAccessException { Line 106: } Line 107: return storedProceduresMap.get(procName); Line 108: } Line 109: Line 110: private void fillProcMetaData(String procName, Connection con, This method is huge, is it possible to break it don into smaller (logically contained) methods? Line 111: StoredProcedureMetaData procMetaData) throws SQLException { Line 112: SimpleJdbcCall call = getCall(procName, Line 113: createCallForModification(procName)); Line 114: .................................................... File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java Line 84: } Line 85: getCallsHandler().executeStoredProcAsBatch("Updatevds_interface_statistics", Line 86: executions); Line 87: } catch (Exception e) { Line 88: log.error("Can't update Vds Statistics", e); Why catch & log? this is not what is done in other methods, and you're changing the behaviour.. Line 89: } Line 90: } Line 91: Line 92: /** -- 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