Yair Zaslavsky has posted comments on this change.
Change subject: core: support materialized views in Java
......................................................................
Patch Set 4: (5 inline comments)
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/materializedviews/MatrializedViewListener.java
Line 11:
Line 12: public class MatrializedViewListener extends QueuedUpdateListener {
Line 13: private static final Log log =
LogFactory.getLog(MatrializedViewListener.class);
Line 14:
Line 15: public void handleUpdateData(UpdateData data) {
I think that UpdateData should just contain the materialized views names.
The isAnnotationPresent part should be when you submit an element on the queue,
and not at the listener part.
Line 16: if (data.getDao() == null ||
!data.getDao().getClass().isAnnotationPresent(MaterializedView.class)) {
Line 17: return;
Line 18: }
Line 19:
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/updateListeners/QueuedUpdateListener.java
Line 12: private static final int DEFAULT_UPDATE_DELAY = 5 * 1000;
Line 13:
Line 14: private boolean jobScheduled = false;
Line 15: private int updateDelayInterval;
Line 16: protected List<UpdateData> pendingUpdates = new ArrayList<>();
Should't this be synchronized? i.e - BlockingQueue?
Line 17:
Line 18: public QueuedUpdateListener() {
Line 19: updateDelayInterval = DEFAULT_UPDATE_DELAY;
Line 20: }
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/updateListeners/UpdateData.java
Line 2:
Line 3: import org.ovirt.engine.core.dao.DAO;
Line 4:
Line 5: public class UpdateData {
Line 6: private DAO dao;
See previous comment.
Not sure why we need the method name, can you please explain?
Line 7: private String methodName;
Line 8: private Object[] args;
Line 9:
Line 10: public DAO getDao() {
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/updateListeners/UpdateManager.java
Line 11: private static final UpdateManager instance = new UpdateManager();
Line 12:
Line 13: private List<UpdateListener> updateListeners = new ArrayList<>();
Line 14:
Line 15: private ExecutorService executor = Executors.newFixedThreadPool(1);
Use ThreadPoolUtils please
Line 16:
Line 17: private UpdateManager() {
Line 18:
Line 19: }
Line 22: return instance;
Line 23: }
Line 24:
Line 25: public void informUpdate(final DAO originatingDao, final String
method, final Object[] args) {
Line 26: executor.execute(new Runnable() {
Will look shorter when you will use ThreadPoolUtils ;)
Line 27:
Line 28: @Override
Line 29: public void run() {
Line 30: for (UpdateListener listener : updateListeners) {
--
To view, visit http://gerrit.ovirt.org/16305
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I745e934fde341bd1264e5e5fedaf59fc64ad4ad8
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
