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 <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to