Murtadha Hubail has posted comments on this change. Change subject: Feed Fixes and Cleanup ......................................................................
Patch Set 5: (78 comments) https://asterix-gerrit.ics.uci.edu/#/c/574/5//COMMIT_MSG Commit Message: Line 14: 5. Fixed the handling of duplicate key exception but now we have we don't have a deadlock anymore :) https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/ExternalLibraryBootstrap.java File asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/ExternalLibraryBootstrap.java: Line 40: import org.apache.asterix.external.api.IDataSourceAdapter.AdapterType; remove unused import. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/FeedBootstrap.java File asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/FeedBootstrap.java: Line 33: + builder.append("create dataverse " + FeedConstants.FEEDS_METADATA_DV + ";" + "\n")); why are these dataverse created? Line 52: builder.append( why are these dataverse created? https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java File asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java: Line 287: // get node stores Remove comment and format the class. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-common/src/main/java/org/apache/asterix/common/api/INodeFeedManager.java File asterix-common/src/main/java/org/apache/asterix/common/api/INodeFeedManager.java: Line 29: public String getNodeId(); This is not needed in this interface. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-common/src/main/java/org/apache/asterix/common/config/AsterixPropertiesAccessor.java File asterix-common/src/main/java/org/apache/asterix/common/config/AsterixPropertiesAccessor.java: Line 2: * Licensed to the Apache Software Foundation (ASF) under one Reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-common/src/main/java/org/apache/asterix/common/parse/ITupleForwarder.java File asterix-common/src/main/java/org/apache/asterix/common/parse/ITupleForwarder.java: Line 43: public void addTuple(ArrayTupleBuilder tb) throws HyracksDataException, InterruptedException; I don't think InterruptedException is needed. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java File asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java: Line 2: * Licensed to the Apache Software Foundation (ASF) under one Reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataFlowController.java File asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataFlowController.java: Line 41: * 5. start(writer) add new events Line 46: public boolean stop() throws Exception; change exceptions to HyracksDataException https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedRecordDataFlowController.java File asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedRecordDataFlowController.java: Line 34: protected MutableBoolean closed = new MutableBoolean(false); change to atomicboolean Line 56: } move closing to finally. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedStreamDataFlowController.java File asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedStreamDataFlowController.java: Line 44: tupleForwarder.close(); move close to finally https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedTupleForwarder.java File asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedTupleForwarder.java: Line 46: this.appender = new FrameTupleAppender(); pass frame in constructor Line 67: throw new IllegalStateException(); add message here. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/AdapterIdentifier.java File asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/AdapterIdentifier.java: Line 6: * to you under the Apache License, Version 2.0 (the reformat Line 28: private final String namespace; // what is the adapter namespace? dataverse Line 29: private final String name; // what is the adapter name? adapterName https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/GenericAdapter.java File asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/GenericAdapter.java: Line 57: return controller.pause(); resume https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IAdapterExecutor.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IAdapterExecutor.java: Line 23: public interface IAdapterExecutor { Unused interface. Remove https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IFeedOperatorOutputSideHandler.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IFeedOperatorOutputSideHandler.java: Line 27: * and we can probably remove it completely. I should +2 the change just to get this in :) https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IPrimaryFeed.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IPrimaryFeed.java: Line 24: I don't think we need the interface, just add these methods to IFeed Line 25: public Map<String, String> getAdaptorConfiguration(); change to Adapter Line 27: public String getAdaptorName(); change to Adapter https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/CollectTransformFeedFrameWriter.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/CollectTransformFeedFrameWriter.java: Line 2: * Licensed to the Apache Software Foundation (ASF) under one reformat Line 53: tupleAppender = new FrameTupleAppender(); just pass the frame to the constructor https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/DataBucket.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/DataBucket.java: Line 6: * to you under the Apache License, Version 2.0 (the reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedExceptionHandler.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedExceptionHandler.java: Line 39: private final FrameTupleAccessor fta; Add TODO to re-enable logging https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameCollector.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameCollector.java: Line 4: * distributed with this work for additional information reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java: Line 8: * with the License. You may obtain a copy of the License at reformat Line 86: this.connectionId = connectionId; call one constructor from the other https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedActivityIdFactory.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedActivityIdFactory.java: Line 6: * to you under the Apache License, Version 2.0 (the reformat Line 21: public class FeedActivityIdFactory { Remove this class https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/FeedRuntimeReport.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/FeedRuntimeReport.java: Line 21: public class FeedRuntimeReport { Remove! https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/IngestionRuntime.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/IngestionRuntime.java: Line 5: * regarding copyright ownership. The ASF licenses this file Reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/FileSystemFeedProgressTracker.java File asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/FileSystemFeedProgressTracker.java: Line 19: public class FileSystemFeedProgressTracker { Remove! https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/TwitterPushRecordReader.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/TwitterPushRecordReader.java: Line 41: private boolean closed; = false; https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/AInputStreamReader.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/AInputStreamReader.java: Line 36: @Override Remove, let the call go to super https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/LocalFSInputStreamProvider.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/LocalFSInputStreamProvider.java: Line 3: * or more contributor license agreements. See the NOTICE file Reformat Line 45: feedLogFileSplits[partition].getLocalFile().getFile().getPath(), 0, ctx.getIOManager()).getFile(); You need to pass the IO device of the partition from the FileSplit Line 58: } catch (Exception e) { You don't need to wrap the exception here https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/LocalFileSystemInputStream.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/LocalFileSystemInputStream.java: Line 84: public int available() throws IOException { You don' need to override this. Line 90: if (in == null) { Throw unsupported operation and msg to use the read with parameters https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/factory/LocalFSInputStreamProviderFactory.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/factory/LocalFSInputStreamProviderFactory.java: Line 7: * "License"); you may not use this file except in compliance Reformat Line 99: if (!trimmedValue.contains("://")) { If this condition is being checking in multiple locations, please move it to a util along with the exception message. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java File asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java: Line 4: * distributed with this work for additional information reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/library/ResultCollector.java File asterix-external-data/src/main/java/org/apache/asterix/external/library/ResultCollector.java: Line 8: * with the License. You may obtain a copy of the License at reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java File asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java: Line 5: * regarding copyright ownership. The ASF licenses this file reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/operators/ExternalDatasetIndexesRecoverOperatorDescriptor.java File asterix-external-data/src/main/java/org/apache/asterix/external/operators/ExternalDatasetIndexesRecoverOperatorDescriptor.java: Line 4: * distributed with this work for additional information reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedCollectOperatorNodePushable.java File asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedCollectOperatorNodePushable.java: Line 5: * regarding copyright ownership. The ASF licenses this file reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMessageOperatorNodePushable.java File asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMessageOperatorNodePushable.java: Line 6: * to you under the Apache License, Version 2.0 (the reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/provider/AdapterFactoryProvider.java File asterix-external-data/src/main/java/org/apache/asterix/external/provider/AdapterFactoryProvider.java: Line 5: * regarding copyright ownership. The ASF licenses this file reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/DataflowUtils.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/DataflowUtils.java: Line 50: if (ExternalDataUtils.isFeed(configuration)) { Add a TODO to pass this value in the configuration and avoid this check for feeds https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataCompatibilityUtils.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataCompatibilityUtils.java: Line 105: if (adapterClassname.equalsIgnoreCase(ExternalDataConstants.ALIAS_FILE_FEED_ADAPTER)) { change equals https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataConstants.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataConstants.java: Line 5: * regarding copyright ownership. The ASF licenses this file reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataExceptionUtils.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataExceptionUtils.java: Line 33: public static String concat(String... vals) { This is a sophisticated method but it isn't used :) https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataUtils.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataUtils.java: Line 5: * regarding copyright ownership. The ASF licenses this file reformat Line 238: throws AsterixException { Remove exception Line 246: public static boolean waitForData(Map<String, String> configuration) { rename this method Line 248: return true; return false; https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedConstants.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedConstants.java: Line 9: * http://www.apache.org/licenses/LICENSE-2.0 reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedFrameUtil.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedFrameUtil.java: Line 34: public static ByteBuffer getSlicedFrame(IHyracksTaskContext ctx, int tupleIndex, FrameTupleAccessor fta) This was replaced by the method below. Please remove it. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedLogManager.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedLogManager.java: Line 5: * regarding copyright ownership. The ASF licenses this file reformat Line 112: new File(dir.toAbsolutePath().toString() + File.separator + PROGRESS_LOG_FILE_NAME).createNewFile(); use the constructor with two arguments. https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedUtils.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedUtils.java: Line 6: * to you under the Apache License, Version 2.0 (the reformat Line 37: public static String getFeedPointKeyRep(IFeed feed, List<String> appliedFunctions) { Unused method Line 78: public static FileReference getAbsoluteFileRef(String relativePath, int ioDeviceId, IIOManager ioManager) { This needs to be moved to a different util https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java: Line 7: * "License"); you may not use this file except in compliance reformat Line 101: if (it == null) { } https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java: Line 33: public static void traverse(final LinkedList<File> files, File root, final String expression, See if you can replace this method by FileUtils.iterateFilesAndDirs(.) https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/test/java/org/apache/asterix/external/library/UpperCaseFunction.java File asterix-external-data/src/test/java/org/apache/asterix/external/library/UpperCaseFunction.java: Line 8: * with the License. You may obtain a copy of the License at reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/api/IMetadataEntity.java File asterix-metadata/src/main/java/org/apache/asterix/metadata/api/IMetadataEntity.java: Line 26: public interface IMetadataEntity<E> extends Serializable { Please change it to T https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataBootstrap.java File asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataBootstrap.java: Line 3: * or more contributor license agreements. See the NOTICE file reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java File asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java: Line 5: * regarding copyright ownership. The ASF licenses this file reformat https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/MetadataFeedUtil.java File asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/MetadataFeedUtil.java: Line 6: * to you under the Apache License, Version 2.0 (the reformat Line 91: public class MetadataFeedUtil { FeedMetadataUtil https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-om/src/main/java/org/apache/asterix/om/util/AsterixClusterProperties.java File asterix-om/src/main/java/org/apache/asterix/om/util/AsterixClusterProperties.java: Line 3: * or more contributor license agreements. See the NOTICE file Checkout this file from master -- To view, visit https://asterix-gerrit.ics.uci.edu/574 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4e8db26a810efd1fbaa52ceeb3efd0c8328ab070 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-HasComments: Yes
