Oved Ourfali has posted comments on this change. Change subject: engine : Introduction of Poller, CommandEntity and interfaces for caching ......................................................................
Patch Set 5: (3 comments) See some comments about the Cache. I guess you're using it in the next patches. I didn't understand what's the key of this cache, as you use stepId when inserting, and commandId when fetching. If the key should be commandId, then the code may cause inserting the same command more than once. Also, shouldn't you clear entries from the cache when the command / step is cleared? Perhaps I'm missing something here... http://gerrit.ovirt.org/#/c/26335/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java: Line 23: } Line 24: Line 25: @Override Line 26: public void put(Guid stepId, Guid commandId, Guid parentCommandId, SPMAsyncTask task) { Line 27: commandMap.put(stepId, buildGetCommandEntity(commandId, parentCommandId, task)); So the key here is the step id? and you do a get using the commandId? Line 28: } Line 29: Line 30: private CommandEntity buildGetCommandEntity(Guid commandId, Guid parentCommandId, SPMAsyncTask task) { Line 31: CommandEntity entity = new CommandEntity(); Line 26: public void put(Guid stepId, Guid commandId, Guid parentCommandId, SPMAsyncTask task) { Line 27: commandMap.put(stepId, buildGetCommandEntity(commandId, parentCommandId, task)); Line 28: } Line 29: Line 30: private CommandEntity buildGetCommandEntity(Guid commandId, Guid parentCommandId, SPMAsyncTask task) { shouldn't it be buildCommandEntity? Line 31: CommandEntity entity = new CommandEntity(); Line 32: entity.setId(commandId); Line 33: entity.setParentCommandId(parentCommandId); Line 34: CommandEntityUtils.setParameters(entity, task.getParameters().getDbAsyncTask().getActionParameters()); http://gerrit.ovirt.org/#/c/26335/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/CommandEntity.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/CommandEntity.java: Line 63: private Guid commandId; Line 64: private Guid parentCommandId; Line 65: private VdcActionType commandType; Line 66: private Map<String, Object> data; Line 67: private Date createdAt; I'd move the members to the beginning of the class. Line 68: Line 69: public Set<Guid> getChildCommandIds() { Line 70: return childCommandIds; Line 71: } -- To view, visit http://gerrit.ovirt.org/26335 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2172333255131652bb78623352fb805d16fb4a1b Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Greg Padgett <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
