asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yingyi Bu (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Introduce IComponentProvider
Date Wed, 01 Feb 2017 04:46:59 GMT
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 <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message