asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Cleanup and bug fixes in Feeds pipeline
Date Wed, 01 Mar 2017 22:51:12 GMT
abdullah alamoudi has posted comments on this change.

Change subject: Cleanup and bug fixes in Feeds pipeline
......................................................................


Patch Set 9:

(9 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1523/9//COMMIT_MSG
Commit Message:

PS9, Line 7: fixes
> Make the commit message more concrete?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IAdapterFactory.java
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/api/IAdapterFactory.java:

PS9, Line 87: IExternalDataSourceFactory
> Document the new method in the interface, e.g., why do we need this?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/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:

PS9, Line 72: message
> What's the purpose of the null message?  Add some comments to explain that?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/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:

PS9, Line 591: AlgebricksException
> Error code?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java:

PS9, Line 199: lsmAccessor
> enter/exist used to happen for each individual modification.  What's the mo
There are two motivations for this.
1. Some update pipelines might need to update the metadata page of the memory component and
they need to ensure it is entered even in the absence of records in the frame
2. Future optimization for operations that can be done with a single enter operation per frame.


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/runtime/UpsertCommitRuntime.java
File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/runtime/UpsertCommitRuntime.java:

PS9, Line 30: protected
> why protected?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/PartitionDataWriter.java
File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/PartitionDataWriter.java:

PS9, Line 41: protected
> why protected?
accessed by subclasses that change the behavior of nextFrame


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMHarness.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMHarness.java:

PS9, Line 70: enter
> What's the motivation of exposing enter and exit?  I think they're implemen
The motivation is to allow entering a component for the purpose of protecting the component
against certain IO operation. the caller for this method is responsible for ensuring that
exit gets called. they would do that as follows:

enter(ctx);
try{
} finally{
  exit(ctx);
}


https://asterix-gerrit.ics.uci.edu/#/c/1523/9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java:

PS9, Line 169: enterMemoryComponent
> expose enter/exist as public methods seem dangerous?  You don't have the co
That is true. the caller must ensure the component is exited. There is a use case which needs
the ability to enter the memory component so it can update the memory metadata page knowing
that it will not be flushed while it is being updated.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1523
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie97b2133ebecb7380cf0ba336e60ed714d06f8ee
Gerrit-PatchSet: 9
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message