abdullah alamoudi has posted comments on this change. Change subject: Cleanup Feed CodeBase ......................................................................
Patch Set 3: (66 comments) https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/servlet/FeedServletUtil.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/servlet/FeedServletUtil.java: Line 29: > File an issue to restore feed web console Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/FeedLifecycleListener.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/FeedLifecycleListener.java: Line 156: return null; > return empty set. and fix caller not to check for null Done Line 160: public Set<IClusterManagementWork> notifyNodeJoin(String joinedNodeId) { > same as before Done Line 165: public void notifyRequestCompletion(IClusterManagementWorkResponse response) { > stopped using Done Line 169: public void notifyStateChange(ClusterState previousState, ClusterState newState) { > we don't use this anymore Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/FeedOperations.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/FeedOperations.java: Line 138: public static Pair<IOperatorDescriptor, AlgebricksPartitionConstraint> buildDiscontinueFeedMessengerRuntime( > remove unused Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/FeedWorkCollection.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/FeedWorkCollection.java: Line 82: PrintWriter writer = new PrintWriter(System.out, true); > write to a string and log the output Done Line 116: ((SubscribeFeedWork) work).request.setSubscriptionStatus(ConnectionStatus.ACTIVE); > wrong level Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/AsterixLSMInsertDeleteOperatorNodePushable.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/AsterixLSMInsertDeleteOperatorNodePushable.java: Line 120: th.printStackTrace(); > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/pom.xml File asterixdb/asterix-external-data/pom.xml: Line 197: <groupId>org.apache.hyracks</groupId> > scope test Done Line 303: <groupId>org.mockito</groupId> > scope test Done Line 308: <groupId>org.powermock</groupId> > file and issue for license and notice for powermock Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IExternalIndexer.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IExternalIndexer.java: Line 29: @SuppressWarnings("deprecation") > remove it Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IRecordWithMetaDataAndPKParser.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IRecordWithMetaDataAndPKParser.java: Line 27: @SuppressWarnings("deprecation") > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IRecordWithPKDataParser.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IRecordWithPKDataParser.java: Line 25: @SuppressWarnings("deprecation") > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/ITupleForwarder.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/ITupleForwarder.java: Line 27: public interface ITupleForwarder { > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/AbstractFeedDataFlowController.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/AbstractFeedDataFlowController.java: Line 29: @SuppressWarnings("deprecation") > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/ChangeFeedDataFlowController.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/ChangeFeedDataFlowController.java: Line 31: @SuppressWarnings("deprecation") > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/ChangeFeedWithMetaDataFlowController.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/ChangeFeedWithMetaDataFlowController.java: Line 32: public class ChangeFeedWithMetaDataFlowController<T, O> extends FeedWithMetaDataFlowController<T, O> { > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/CounterTimerTupleForwarder.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/CounterTimerTupleForwarder.java: Line 37: @SuppressWarnings("deprecation") > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedTupleForwarder.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedTupleForwarder.java: Line 110: public void close() throws HyracksDataException { > Fix this using finally Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IExceptionHandler.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IExceptionHandler.java: Line 25 > put back Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IFeedRuntime.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IFeedRuntime.java: Line 39: DISCARD // Memory budget has been consumed. Disk space budget has been consumed. Now we're discarding > too long Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/DistributeFeedFrameWriter.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/DistributeFeedFrameWriter.java: Line 75: * @throws HyracksDataException > WS Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedExceptionHandler.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedExceptionHandler.java: Line 78: //TODO: Fix logging of exceptions > log using the logger for now Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameCollector.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameCollector.java: Line 84: public void setFrameWriter(IFrameWriter nextOnlyWriter) { > change the name Done Line 114: th.printStackTrace(); > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameSpiller.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameSpiller.java: Line 42: * cannot process incoming data at its arrival rate. The maximum size of data (tuples) that can be spilled to disk is configured using the property > reformat the comment to fit 120 char width Done Line 68: this.connectionId = connectionId; > make generic and not feed related Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java: Line 109: try { > join thread then close writer Done Line 126: try { > just make it volatile Done Line 205: synchronized (spiller) { > make it a constant instead of number Done Line 265: notify(); > should return; Done Line 319: private void consume(ByteBuffer frame) { > change to return the cause instead of void Done Line 341: memoryManager.release(frame); > put release in a local finally clause Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FrameDistributor.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FrameDistributor.java: Line 63: collector.nextFrame(frame); > Fix. What should be done?: Done Line 88: try { > handle failure as in nextFrame() Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/SyncFeedRuntimeInputHandler.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/SyncFeedRuntimeInputHandler.java: Line 51: throw new HyracksDataException(th); > change from throwable to hde in the catch. not just here. Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedMemoryManager.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedMemoryManager.java: Line 34: private static final String ERROR_INVALID_FRAME_SIZE = "The size should be an integral multiple of the default frame size"; > long line Done Line 41: private int handed; > handedOut Done Line 53: if (handed + 1 <= budget) { > handed < budget Done Line 68: int multiples = bufferSize / defaultFrameSize; > rename to multiplier Done Line 73: largeFramesPool = new ArrayDeque<>(); > only create the pool when you want to release. Done Line 93: > private Done Line 124: } catch (Throwable th) { > remove try catch Done Line 176: // check subscribers > loop until you can't satisfy request Done Line 209: // none of the above, add to subscribers and return true > WSs Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/CollectionRuntime.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/CollectionRuntime.java: Line 55: if (!(isCollectionOver())) { > change to interrupted exception Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/FeedRuntimeId.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/FeedRuntimeId.java: Line 30: public static final String DEFAULT_OPERAND_ID = "N/A"; > remove dataset. add comment as to what is operand. then fix constructor Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/SubscribableRuntime.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/SubscribableRuntime.java: Line 56: public synchronized void subscribe(CollectionRuntime collectionRuntime) throws HyracksDataException { > This is never called. Should we remove? Done Line 63: public synchronized void unsubscribe(CollectionRuntime collectionRuntime) throws HyracksDataException { > also unsubscribe is never called Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedIntakeOperatorDescriptor.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedIntakeOperatorDescriptor.java: Line 107: private IAdapterFactory createExtenralAdapterFactory(IHyracksTaskContext ctx, int partition) throws Exception { > typo Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedIntakeOperatorNodePushable.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedIntakeOperatorNodePushable.java: Line 73: Thread.currentThread().setName("Intake Thread"); > remove renaming of thread Done Line 118: } > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMetaStoreNodePushable.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMetaStoreNodePushable.java: Line 156: FeedUtils.processFeedMessage(buffer, message, fta); > ensure consistency between feed operators' members names. try consolidate b Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/feed/test/FeedSpillerUnitTest.java File asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/feed/test/FeedSpillerUnitTest.java: Line 96: * Check that we have 3 files. > WSs Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java: Line 2214: @SuppressWarnings("rawtypes") > remove Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/FrameHelper.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/FrameHelper.java: Line 40: outputFrame.array()[start + FrameConstants.META_DATA_FRAME_COUNT_OFFSET] = (byte) (numberOfMinFrame & 0xff); > remove comment (even though comment is good) Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/IHyracksFrameMgrContext.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/IHyracksFrameMgrContext.java: Line 39: * @param bytes > Revert and submit a JIRA issue instead. Done Line 40: * > WS Done Line 44: * > WS and line too long Done Line 46: * > WS Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/resources/memory/FrameManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/resources/memory/FrameManager.java: Line 51: } > revert Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/io/MessagingFrameTupleAppender.java File hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/io/MessagingFrameTupleAppender.java: Line 64: public void appendMessage() { > make private and make message a parameter Done Line 74: } catch (Throwable th) { > remove try catch Done https://asterix-gerrit.ics.uci.edu/#/c/798/3/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/LocalityAwareMToNPartitioningConnectorDescriptor.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/LocalityAwareMToNPartitioningConnectorDescriptor.java: Line 56: * (org.apache.hyracks.api.context.IHyracksTaskContext, > revert Done -- To view, visit https://asterix-gerrit.ics.uci.edu/798 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I545bc4f8560564e4c868a80d27c77a4edd97a8b8 Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
