You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by "Yingyi Bu (Code Review)" <do...@asterixdb.incubator.apache.org> on 2017/02/01 04:46:59 UTC
Change in asterixdb[master]: Introduce IComponentProvider
Yingyi Bu has posted comments on this change.
Change subject: Introduce IComponentProvider
......................................................................
Patch Set 14:
(69 comments)
https://asterix-gerrit.ics.uci.edu/#/c/1451/4/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/base/ILangExtension.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/base/ILangExtension.java:
Line 59
Why this static method is put in this interface?
https://asterix-gerrit.ics.uci.edu/#/c/1451/4/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/ExternalDataLookupPOperator.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/ExternalDataLookupPOperator.java:
Line 77:
renaming? retainNull -> retainMissing
https://asterix-gerrit.ics.uci.edu/#/c/1451/4/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ValidateUtil.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ValidateUtil.java:
Line 169: * a map of key fields that will be validated
Code style?
https://asterix-gerrit.ics.uci.edu/#/c/1451/4/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/java/AsterixJavaClient.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/java/AsterixJavaClient.java:
Line 75: }
code style?
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/RecoveryManager.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/RecoveryManager.java:
PS14, Line 216: (
Reduce side effects:
analyzeJobCommit(winnerJobSet, logRecord);
->
jobId = logRecord.getJobId();
winnerJobSet.add(jobId);
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/TransactionSubsystem.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/TransactionSubsystem.java:
PS14, Line 174: Thread
When can the while condition return false?
If the thread is interrupted, Thread.sleep() will throw an InterruptedException and go out of the method.
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-app/src/main/java/org/apache/asterix/file/ComponentProvider.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/file/ComponentProvider.java:
PS14, Line 36: ComponentProvider
ComponentProvider --> StorageComponentProvider
PS14, Line 36: IComponentProvider
IComponentProvider --> IStorageComponentProvider
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-app/src/main/java/org/apache/asterix/util/DataverseUtils.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/util/DataverseUtils.java:
PS14, Line 19: util
util -> utils
PS14, Line 31: DataverseUtils
DataverseUtil -> DataverseUtil
PS14, Line 39: splitAndConstraints
splitAndConstraints
-->
getSplitProviderAndConstraints
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-app/src/main/java/org/apache/asterix/util/ExtensionUtils.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/util/ExtensionUtils.java:
PS14, Line 35: ExtensionUtils
ExtensionUtils -> ExtensionUtil
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-app/src/test/java/org/apache/asterix/api/http/servlet/ConnectorApiLetTest.java
File asterixdb/asterix-app/src/test/java/org/apache/asterix/api/http/servlet/ConnectorApiLetTest.java:
PS14, Line 172: ComponentProvider
use null here since the component provider shouldn't be reached from the findType(...) call.
https://asterix-gerrit.ics.uci.edu/#/c/1451/13/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/optimizer/OptimizerTest.java
File asterixdb/asterix-app/src/test/java/org/apache/asterix/test/optimizer/OptimizerTest.java:
Line 81: protected static IComponentProvider storageManager = new ComponentProvider();
storageManger - > componentProvider
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/optimizer/OptimizerTest.java
File asterixdb/asterix-app/src/test/java/org/apache/asterix/test/optimizer/OptimizerTest.java:
PS14, Line 81: storageManager
storageManager --> storageComponentProvider
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IAppRuntimeContext.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IAppRuntimeContext.java:
PS14, Line 116: getComponentProvider
getComponentProvider() --> getStorageComponentProvider()
https://asterix-gerrit.ics.uci.edu/#/c/1451/13/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/IComponentProvider.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/IComponentProvider.java:
Line 30: * Responsible for runtime components
The comments seems confusing. The artifacts that returned from this class are mostly used at compiler side and recovery?
I guess the old name makes sense to me IStorageComponentProvider.
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/IComponentProvider.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/IComponentProvider.java:
PS14, Line 30: runtime
runtime components
-->
storage components.
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/IApplicationContextInfo.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/IApplicationContextInfo.java:
PS14, Line 45: getRuntimeComponentProvider
getRuntimeComponentProvider --> getStorageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/LSMInvertedIndexInsertDeleteOperatorDescriptor.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/LSMInvertedIndexInsertDeleteOperatorDescriptor.java:
PS14, Line 47: runtimeComponentProvider
runtimeComponentProvider --> storageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/LSMTreeInsertDeleteOperatorDescriptor.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/LSMTreeInsertDeleteOperatorDescriptor.java:
PS14, Line 50: LSMTreeInsertDeleteOperatorDescriptor
runtimeComponentProvider --> storageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/ITransactionSubsystem.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/ITransactionSubsystem.java:
PS14, Line 25: level
responsible -> is responsible
level -> level.
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/util/ConfigUtil.java
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/util/ConfigUtil.java:
PS14, Line 19: util
util -> utils
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/IndexInfoOperatorDescriptor.java
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/IndexInfoOperatorDescriptor.java:
PS14, Line 72: getRuntimeComponentProvider
getRuntimeComponentProvider --> getStorageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java:
PS14, Line 157: componentProvider
storageComponentProvider
PS14, Line 1100: jobId
remove the useless jobId.getId() ?
PS14, Line 1692: getRuntimeComponentProvider
getRuntimeComponentProvider --> getStorageComponentProvider
PS14, Line 2051: splitAndConstraints
splitAndConstraints --> getSplitProviderAndConstraints
PS14, Line 2117: getComponentProvider
getComponentProvider -- > getStorageComponentProvider
https://asterix-gerrit.ics.uci.edu/#/c/1451/4/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java:
Line 428: * if the factory could not be created for the index/dataset combination
"Create the index dataflow helper factory"
-->
"Create the index dataflow helper factory for a particular index on the dataset."
Document parameters, return and throw.
Line 438: return LSMRTreeIOOperationCallbackFactory.INSTANCE;
Code format doesn't seem to follow the code style.
"indexDataflowHelperFactor" --> "getIndexDataflowHelperFactor"
Line 473: * @throws AlgebricksException
Document the method, parameters, return and throws.
Line 476: public ISearchOperationCallbackFactory getSearchCallbackFactory(IComponentProvider storageComponentProvider,
ioOperationCallbackFactory--> getIoOperatortionCallbackFactory.
Line 498: * the index
Document the method, parameters, return and throws.
Line 502: * the operation performed for this callback
indexOperationTrackerProvider --> getIndexOperationTrackerProvider
Line 510: public IModificationOperationCallbackFactory getModificationCallbackFactory(IComponentProvider componentProvider,
Document the method, parameters, return and throws.
Line 516: return op == IndexOperation.UPSERT
searchBacllbackFactory --> getSearchCallbackFactory
Line 537: try {
Document the method, parameters, return and throws.
Line 543: }
modificationCallbackFactory --> getModificationCallbackFactory
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/DatasetLock.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/DatasetLock.java:
PS14, Line 19: util
util -> utils
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/DatasetUtils.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/DatasetUtils.java:
PS14, Line 84: DatasetUtils
DatasetUtils -> DatasetUtil
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/IndexUtils.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/IndexUtils.java:
PS14, Line 19: util
util -> utils
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/KeyFieldTypeUtils.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/KeyFieldTypeUtils.java:
Line 35: public class KeyFieldTypeUtils {
> MAJOR SonarQube violation:
fix this?
PS14, Line 35: KeyFieldTypeUtils
KeyFieldTypeUtils -> KeyFieldUtil
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/MetadataLockManager.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/MetadataLockManager.java:
Line 30: public static MetadataLockManager INSTANCE = new MetadataLockManager();
> MAJOR SonarQube violation:
Fix this?
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/MetadataUtils.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/MetadataUtils.java:
PS14, Line 21: MetadataUtils
MetadataUtils -> MetadataUtil
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/SplitsAndConstraintsUtil.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/SplitsAndConstraintsUtil.java:
PS14, Line 134: dataverseSplitsAndConstraints
dataverseSplitsAndConstraints -> dataverseSplitProviderAndConstraints
PS14, Line 140: filesIndexSplitAndConstraint
filesIndexSplitAndConstraints -> filesIndexSplitProviderAndConstraints
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/TypeUtils.java
File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/util/TypeUtils.java:
PS14, Line 40: TypeUtils
TypeUtils --> TypeUtil
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMInvertedIndexUpsertOperatorDescriptor.java
File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMInvertedIndexUpsertOperatorDescriptor.java:
PS14, Line 57: iPageManagerFactory
iPageManagerFactory -> pageManagerFactory
https://asterix-gerrit.ics.uci.edu/#/c/1451/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMTreeUpsertOperatorDescriptor.java
File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMTreeUpsertOperatorDescriptor.java:
PS4, Line 61: iPageManagerFactory
iPageManagerFactory -> pageManagerFactory
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMTreeUpsertOperatorDescriptor.java
File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMTreeUpsertOperatorDescriptor.java:
PS14, Line 61: iPageManagerFactory)
iPageManagerFactory -> pageManagerFactory
https://asterix-gerrit.ics.uci.edu/#/c/1451/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/util/AppContextInfo.java
File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/util/AppContextInfo.java:
PS4, Line 181: aryManager getLibraryMan
avoid the setter method, and instead, passing that through the constructor.
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreeclient/src/main/java/org/apache/hyracks/examples/btree/client/SecondaryIndexSearchExample.java
File hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreeclient/src/main/java/org/apache/hyracks/examples/btree/client/SecondaryIndexSearchExample.java:
PS14, Line 101: RuntimeContextComponentProvider
RuntimeContextComponentProvider -> StorageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContextComponentProvider.java
File hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContextComponentProvider.java:
PS14, Line 29: RuntimeContextComponentProvider
RuntimeContextComponentProvider -> StorageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/AbstractBTreeOperatorTest.java
File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/AbstractBTreeOperatorTest.java:
PS14, Line 74: TestRuntimeComponentProvide
TestRuntimeComponentProvider -> TestStorageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/dataflow/BTreeDataflowHelper.java
File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/dataflow/BTreeDataflowHelper.java:
PS14, Line 48: getRuntimeComponentProvider
getRuntimeComponentProvider -> getStorageComponentProvider
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/AbstractIndexOperatorDescriptor.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/AbstractIndexOperatorDescriptor.java:
PS14, Line 41: runtimeComponentProvider;
runtimeComponentProvider -> storageManager
PS14, Line 64: runtimeComponentProvider
runtimeComponentProvider -> storageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/AbstractTreeIndexOperatorDescriptor.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/AbstractTreeIndexOperatorDescriptor.java:
PS14, Line 45: runtimeComponentProvider
runtimeComponentProvider -> storageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IIndexOperatorDescriptor.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IIndexOperatorDescriptor.java:
PS14, Line 37: getRuntimeComponentProvider
getRuntimeComponentProvider -> getStorageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/dataflow/LSMTreeIndexCompactOperatorDescriptor.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/dataflow/LSMTreeIndexCompactOperatorDescriptor.java:
PS14, Line 44: runtimeComponentProvider
runtimeComponentProvider -> storageManager
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/dataflow/RTreeDataflowHelper.java
File hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/dataflow/RTreeDataflowHelper.java:
PS14, Line 52: etRuntimeComponentProvider()
getRuntimeComponentProvider() -> getStorageManager()
PS14, Line 53: getRuntimeComponentProvider()
getRuntimeComponentProvider() -> getStorageManager()
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/IStorageManager.java
File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/IStorageManager.java:
PS14, Line 36: disk
disk buffer cache
PS14, Line 43: root
root?
PS14, Line 50: root
root?
PS14, Line 57: root
root?
https://asterix-gerrit.ics.uci.edu/#/c/1451/14/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestRuntimeComponentProvider.java
File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestRuntimeComponentProvider.java:
PS14, Line 28: TestRuntimeComponentProvider
TestRuntimeComponentProvider -> TestStorageManager
--
To view, visit https://asterix-gerrit.ics.uci.edu/1451
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If86750cdb2436c713f6598e54d4aaaf23d9f7bbf
Gerrit-PatchSet: 14
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <ba...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sj...@ucr.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes