Yair Zaslavsky has posted comments on this change.
Change subject: gluster: Gluster Sync job event log corrected
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java
Line 578: @SuppressWarnings("serial")
Line 579: private void updateExistingAndNewOptions(final
GlusterVolumeEntity existingVolume,
Line 580: Collection<GlusterVolumeOptionEntity> fetchedOptions) {
Line 581:
Line 582: Map<String, GlusterVolumeOptionEntity> existingOptions = new
HashMap<String, GlusterVolumeOptionEntity>();
Feel free to move to JDK7 generic syntax here (though not mandatory for merge)
Line 583: Map<String, GlusterVolumeOptionEntity> newOptions = new
HashMap<String, GlusterVolumeOptionEntity>();
Line 584:
Line 585: for (final GlusterVolumeOptionEntity fetchedOption :
fetchedOptions) {
Line 586: final GlusterVolumeOptionEntity existingOption =
existingVolume.getOption(fetchedOption.getKey());
Line 604:
TransactionSupport.executeInScope(TransactionScopeOption.Required,
Line 605: new TransactionMethod<Void>() {
Line 606: @Override
Line 607: public Void runInTransaction() {
Line 608: saveNewOptions(existingVolume,
newOptionsSortedList);
Can you please elaborate how frequent this will be run + amount of update
operations?
Maybe worth waiting for our work on spring-jdbc-batch if possible?
Line 609: return null;
Line 610: }
Line 611: });
Line 612: }
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJobTest.java
Line 399: return new
ArgumentMatcher<Collection<GlusterVolumeOptionEntity>>() {
Line 400:
Line 401: @Override
Line 402: public boolean matches(Object argument) {
Line 403: if (!(argument instanceof Collection<?>)) {
I see that one method above you check instancof ArrayList.
Is it ok that you check here instanceof Collection?
Line 404: return false;
Line 405: }
Line 406: @SuppressWarnings("unchecked")
Line 407: ArrayList<GlusterVolumeOptionEntity> optionsToAdd =
(ArrayList<GlusterVolumeOptionEntity>) argument;
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MassOperationsGenericDaoDbFacade.java
Line 47: }
Line 48:
Line 49: @Override
Line 50: public void saveAll(Collection<T> entities) {
Line 51: for (T entity : entities) {
saveAll not in transaction? maybe wait for our work on spring-jdbc-batch ?
Line 52: save(entity);
Line 53: }
Line 54: }
--
To view, visit http://gerrit.ovirt.org/15172
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82ed05a8ef5faec8202b79fe0037482d956f3fed
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[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
