asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Westmann (Code Review)" <>
Subject Change in asterixdb[master]: WIP
Date Sun, 23 Jul 2017 18:10:48 GMT
Till Westmann has posted comments on this change.

Change subject: WIP

Patch Set 13:

File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/

PS13, Line 169: /**
              :      * @return the storage component provider
              :      */
              :     IStorageComponentProvider getComponentProvider();
              :     /**
              :      * @return the application context
              :      */
              :     ICcApplicationContext getApplicationContext();
This seems to be the wrong interface for these methods. Why would a statement executor expose
the storage component provider or the application context?
File asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/

PS13, Line 58: protected ClusterState state;
It seems that the state is not needed anymore. Is that right? If so, should we remove it?
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/

PS13, Line 58: refreshState
Why rename this? Everything in this interface is about cluster state notifications ... the
new name is confusing.
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/cluster/

PS13, Line 118: setGlobalRecoveryCompleted
Just looking at the names here it seems that this should be on the IGlobalRecoveryManager
and not on the IClusterStateManager.
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/cluster/

PS13, Line 30: HyracksDataException
Document when this newly added exception is raised?
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/

PS13, Line 70: getActiveNotificationHandler
Why this difference between the return type and the method name?

PS13, Line 99: MetadataLockManager
Should we have an interface here?
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/

File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/

PS13, Line 56: 
What was wrong with this approach?
File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/

PS13, Line 33: protected
I agree with SQ here, we should keep exceptions unmodifiable. What was wrong with the previous
File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/application/

PS13, Line 92: , JobStatus jobStatus, List<Exception> exceptions
Can't we get this information from Hyracks' JobManager?
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/tuples/

Line 38:             System.err.println("huh!");
> MAJOR SonarQube violation:
Agreed :)

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifeac8c73e6bad39a13663b84a52121356e3c6b40
Gerrit-PatchSet: 13
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Murtadha Hubail <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: Xikui Wang <>
Gerrit-HasComments: Yes

View raw message