asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xikui Wang (Code Review)" <>
Subject Change in asterixdb[master]: Feed Connection Refactoring
Date Wed, 07 Dec 2016 00:15:55 GMT
Xikui Wang has posted comments on this change.

Change subject: Feed Connection Refactoring

Patch Set 5:


I added several comments to yours. I will try to do some cleaning and merge the updates from
master as well. Thanks!

Sorry for the delay. I was preparing for the candidacy exam preparation.
File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/

Line 116:     private static final ILangCompilationProvider compilationProvider = new AqlCompilationProvider();
> get rid of static compilationProvider
I added two local variables in QueryTranslator before calling getConnectJob. Is that ok?

Line 162:         PrintWriter writer = new PrintWriter(System.err, true);
> MAJOR SonarQube violation:
Any idea about this issue?

Line 197:             ingestionOp = new FeedIntakeOperatorDescriptor(jobSpec, feed, firstOp.getAdaptorLibraryName(),
> when will the adapterFactory be null?
If it's external adaptor from UDF, this will be null. It will be bind to class at execution

Line 232:                 if (opDesc instanceof AsterixLSMTreeInsertDeleteOperatorDescriptor
> I believe that we don't need the if or the else here!!
The reason I didn't remove feed collect operator from the workflow is for feed policy. I couldn't
find a better place to adopt feed policy in current setting, so I kept the feed collect operator....
File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/

Line 345:                     case Statement.Kind.CONNECT_FEED:
> add test cases that queries the connection dataset after connect and after 
There is one test case connect-feed added for this case. I will change it to query for more
detailed connection info.

Line 2143:         try {
> what if feed has already been started?
Fixed this, as well as for stopFeed.
I met one tricky case in this though. If I execute 2 stop statement with small time gap, the
2nd stop will throw an exception as the 1st stop is not fully executed yet. Is that an issue?

Line 2183:             // TODO: check feed is actually running.
> do the todo and add a test case?
Done with the start fix.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic36267eb9a10df21734ce1cc1f38583e23c9e8f0
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Xikui Wang <>
Gerrit-Reviewer: abdullah alamoudi <>
Gerrit-HasComments: Yes

View raw message