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 hyracks[master]: Changed the IFrameWriter Contract
Date Tue, 15 Dec 2015 00:57:02 GMT
Yingyi Bu has posted comments on this change.

Change subject: Changed the IFrameWriter Contract
......................................................................


Patch Set 2:

(42 comments)

https://asterix-gerrit.ics.uci.edu/#/c/551/2/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/PartitioningSplitOperatorDescriptor.java
File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/PartitioningSplitOperatorDescriptor.java:

Line 48:  * This IFrameWriter doesn't follow the contract
Why it doesn't/can't follow the contract? Can't it be fixed?


https://asterix-gerrit.ics.uci.edu/#/c/551/2/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamSelectRuntimeFactory.java
File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamSelectRuntimeFactory.java:

Line 110:             }
should you also overwrite fail()?  Because you have the state isOpen here to indicate whether
you should call writer.fail().


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameWriter.java
File hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameWriter.java:

Line 36:  * processing. Once open() is called, the {@link IFrameWriter} is in the OPENED
"...called, no matter successfully or not, the..."


Line 42:  * <li>{@link IFrameWriter#nextFrame(ByteBuffer)} to provide data to the {@link
IFrameWriter}. The call returns normally on success and the {@link IFrameWriter} remains in
the OPENED state. On failure, the call throws a {@link HyracksDataException}, and the {@link
IFrameWriter} enters the FAILED state.</li>
IMO: if the call throws a {@link HyracksDataException}, the the {@link IFrameWriter} is still
in state OPENED state.  It enters the FAILED state only when its fail() method is called.


Line 48:  * Note: If the call to {@link IFrameWriter#open()} failed, the {@link IFrameWriter#close()}
is still called by the producer.
"is still" --> "should still be"


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-hadoop/src/main/java/org/apache/hyracks/dataflow/hadoop/mapreduce/MapperOperatorDescriptor.java
File hyracks/hyracks-dataflow-hadoop/src/main/java/org/apache/hyracks/dataflow/hadoop/mapreduce/MapperOperatorDescriptor.java:

Line 261:                 } catch (Exception e) {
Exception --> Throwable


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/LocalityAwarePartitionDataWriter.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/LocalityAwarePartitionDataWriter.java:

Line 105:                 } catch (Exception e) {
Exception --> Throwable


Line 107:                         failException = new HyracksDataException("Error during fail()
call to frame writer", e);
if (failException == null) {	
       failException = new HyracksDataException(e);
} else {
       failException.addSuppressed(e);
}

"Error during fail() call to frame writer" doesn't give me any "information gain" --- we can
find this line from the stack trace.


Line 131:                 } catch (Exception e) {
Exception --> Throwable


Line 139:                     } catch (Exception e) {
Exception --> Throwable


Line 142:                         }
if (closeException == null) {	
       closeException = new HyracksDataException(e);
} else {
       closeException.addSuppressed(e);
}

"Error during close() call of writer" doesn't give me any additional "information gain" ---
we can find this line from the stack trace.


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/MToNReplicatingConnectorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/MToNReplicatingConnectorDescriptor.java:

Line 71:                         } catch (Exception e) {
Exception --> Throwable


Line 73:                                 failException = new HyracksDataException("Exception
closing frame writer");
if(failException == null)}{
   failException = new HyracksDataException(e);
} else{
   failException.addSuppressed(e);
}

The message "Exception closing frame writer" doesn't seem interesting because we all know
that from the stack trace:-)


Line 91:                         } catch (Exception e) {
Exception --> Throwable


Line 93:                                 closeException = new HyracksDataException("Exception
closing frame writer");
if(closeException == null)}{
   closeException = new HyracksDataException(e);
} else{
   closeException.addSuppressed(e);
}


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

Line 72:                     } catch (Exception e) {
Exception-->Throwable


Line 75:                         }
if(closeException == null)}{
   closeException = new HyracksDataException(e);
} else{
   closeException.addSuppressed(e);
}

The message ""Failure during close() call"" doesn't seem interesting because we all know that
from the stack trace:-)


Line 81:                 } catch (Exception e) {
Exception --> Throwable


Line 82:                     if (closeException == null) {
if(closeException == null)}{
   closeException = new HyracksDataException(e);
} else{
   closeException.addSuppressed(e);
}

The message ""Failure during close() call"" doesn't seem interesting because we all know that
from the stack trace:-)


Line 130:                 } catch (Exception e) {
Exception --> Throwable


Line 132:                         failException = new HyracksDataException("Error during fail()
call");
if(failException == null)}{
   failException = new HyracksDataException(e);
} else{
   failException.addSuppressed(e);
}

The message ""Error during fail() call"" doesn't seem interesting because we all know that
from the stack trace:-)


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/file/FileScanOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/file/FileScanOperatorDescriptor.java:

Line 69:                 } catch (Exception e) {
Exception -> Throwable


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java:

Line 418:                     writer.open();
move writer.open() to be the first line of this method?


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java:

Line 233:                     writer.open();
move up writer.open() a line?


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java:

Line 197:                     writer.open();
move up writer.open() a line?


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java:

Line 381:                     writer.open();
move up writer.open() a line?


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/MaterializerTaskState.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/MaterializerTaskState.java:

Line 78:         } catch (Exception e) {
Exception-->Throwable


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/SplitOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/SplitOperatorDescriptor.java:

Line 114:              * This IFrameWriter doesn't follow the contract
Can't that be fixed?


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/rewriting/SuperActivityRewritingTest.java
File hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/rewriting/SuperActivityRewritingTest.java:

Line 87:                 } catch (Exception e) {
Exception --> Throwable


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-hdfs/hyracks-hdfs-core/src/main/java/org/apache/hyracks/hdfs/dataflow/HDFSReadOperatorDescriptor.java
File hyracks/hyracks-hdfs/hyracks-hdfs-core/src/main/java/org/apache/hyracks/hdfs/dataflow/HDFSReadOperatorDescriptor.java:

Line 143:                 } catch (Exception e) {
Exception -> Throwable


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-hdfs/hyracks-hdfs-core/src/main/java/org/apache/hyracks/hdfs2/dataflow/HDFSReadOperatorDescriptor.java
File hyracks/hyracks-hdfs/hyracks-hdfs-core/src/main/java/org/apache/hyracks/hdfs2/dataflow/HDFSReadOperatorDescriptor.java:

Line 158:                 } catch (Exception e) {
Exception --> Throwable


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-storage-am-btree/src/test/java/org/apache/hyracks/storage/am/btree/test/FramewriterTest.java
File hyracks/hyracks-storage-am-btree/src/test/java/org/apache/hyracks/storage/am/btree/test/FramewriterTest.java:

Line 148:         // since we have two appenders, then we need to test each test twice 
space


Line 233:          *   
space.


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java:

Line 195:         } catch (Exception e) {
Exception --> Throwable


Line 196:             closeException = new HyracksDataException("Failed pushing frames to
next operator", e);
closeException = new HyracksDataException(e);
"Failed pushing frames to next operator" doesn't seem interesting to me because we can find
this line in the stack trace.


Line 213:                         closeException = new HyracksDataException("Failed to close
next operator", e);
if(closeException == null)}{
   closeException = new HyracksDataException(e);
} else{
   closeException.addSuppressed(e);
}


Line 226:                             throw new HyracksDataException("Failed to close next
operator", e);
if(closeException == null)}{
   closeException = new HyracksDataException(e);
} else{
   closeException.addSuppressed(e);
}

Message "Failed to close next operator" doesn't give me any "information gain":-)


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/TreeIndexDiskOrderScanOperatorNodePushable.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/TreeIndexDiskOrderScanOperatorNodePushable.java:

Line 88:             } catch (Exception e) {
Exception --> Throwable


Line 92:                 if (scanStarted) {
scanStarted doesn't seem to be accurate enough to show the cursor is successfully opened or
not... 
Because diskOrderScan(cursor) internally will open the cursor after a few lines of code..


Line 95:                     } catch(Exception cursorCloseException){
Exception-->Throwable


Line 101:         } catch (Exception e) {
Throwable.


https://asterix-gerrit.ics.uci.edu/#/c/551/2/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/BinaryTokenizerOperatorNodePushable.java
File hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/BinaryTokenizerOperatorNodePushable.java:

Line 38: /*
Why it doesn't follow?  Can it be fixed?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I03da090002f79f4db7b5b31454ce3ac2b9e40c7f
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yingyib@google.com>
Gerrit-HasComments: Yes

Mime
View raw message