asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Enable Feed Changes to work with BAD project
Date Thu, 23 Feb 2017 22:40:02 GMT
abdullah alamoudi has posted comments on this change.

Change subject: Enable Feed Changes to work with BAD project
......................................................................


Patch Set 1:

(3 comments)

Let's think of the case where there are no listeners, Should the job be removed? What if it
is a repeatable job? can the job be registered another time?

The only reason we register that job through the job lifecycle listener is that we don't have
the job Id yet when we have the active entity and listener

https://asterix-gerrit.ics.uci.edu/#/c/1524/1/asterixdb/asterix-active/src/main/java/org/apache/asterix/active/ActiveJobNotificationHandler.java
File asterixdb/asterix-active/src/main/java/org/apache/asterix/active/ActiveJobNotificationHandler.java:

PS1, Line 56: er listener = e
what if there was no listener. this will cause a null pointer exception


PS1, Line 60: if (event.getEventKind() == Kind.JOB_FINISHED && !listener.isRepeatable()
Let's remove the block completely and the listener can remove itself when notified


PS1, Line 75: removeJob
maybe rename to remove job and listener?

make synchronized and then call unregisterListener from inside the method. also make the two
maps concurrent:
this.jobId2ActiveJobInfos = new HashMap<>();
        this.entityEventListeners = new HashMap<>()

Can you also create a JIRA issue to create unit tests for this class? seems too important
to get right and doesn't have unit tests that cover corner cases


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1524
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62184b67aff564475ef9b58790ff96409195b77
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Xikui Wang <xkkwww@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message