-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25242/#review53146
-----------------------------------------------------------


Actually, I like your approach and your patch. In overall, your work looks 
nice. I have two comments. 

One is that indent of some changes are not matched to our coding standards. We 
use 2 space instead of tab. You need to check the coding style settings of your 
IDE.

Although your patch passes all unit tests, real queries including sort in real 
cluster is is failed.


tajo> select * from lineitem order by l_orderkey, l_partkey;
....


default> select * from lineitem order by l_orderkey, l_partkey;
Progress: 0%, response time: 0.187 sec
Progress: 0%, response time: 0.188 sec
Progress: 0%, response time: 0.39 sec
Progress: 0%, response time: 1.432 sec
Progress: 0%, response time: 2.959 sec
Progress: 0%, response time: 4.696 sec
Progress: 0%, response time: 6.056 sec
Progress: 0%, response time: 7.244 sec
Progress: 0%, response time: 8.252 sec
Progress: 0%, response time: 9.255 sec
Progress: 8%, response time: 10.271 sec
Progress: 8%, response time: 11.547 sec
Progress: 8%, response time: 12.831 sec
Progress: 8%, response time: 15.286 sec
Progress: 8%, response time: 16.35 sec
Progress: 9%, response time: 17.368 sec
Progress: 13%, response time: 18.376 sec
Progress: 17%, response time: 19.603 sec
Progress: 17%, response time: 21.271 sec
Progress: 17%, response time: 22.691 sec
Progress: 17%, response time: 23.696 sec
Progress: 23%, response time: 24.698 sec
Progress: 26%, response time: 25.755 sec
Progress: 26%, response time: 26.872 sec
Progress: 26%, response time: 28.194 sec
Progress: 26%, response time: 29.745 sec
Progress: 26%, response time: 30.752 sec
Progress: 29%, response time: 31.761 sec
Progress: 34%, response time: 32.763 sec
Progress: 34%, response time: 33.765 sec
Progress: 34%, response time: 35.072 sec
Progress: 34%, response time: 36.511 sec
Progress: 34%, response time: 37.513 sec
Progress: 36%, response time: 38.515 sec
Progress: 41%, response time: 40.148 sec
Progress: 43%, response time: 41.15 sec
Progress: 48%, response time: 42.422 sec
Progress: 50%, response time: 43.427 sec
Progress: 50%, response time: 44.429 sec
Progress: 50%, response time: 45.431 sec
Progress: 63%, response time: 46.433 sec
Progress: 63%, response time: 47.435 sec
Progress: 66%, response time: 48.441 sec
Progress: 66%, response time: 49.444 sec
Progress: 66%, response time: 50.446 sec
ERROR: 
/tmp/tajo-hyunsik/tmpdir/q_1410488712506_0004/output/1/11_0/output/output (No 
such file or directory)
java.io.FileNotFoundException: 
/tmp/tajo-hyunsik/tmpdir/q_1410488712506_0004/output/1/11_0/output/output (No 
such file or directory)

If you resolve two issues, this patch would be ready to be committed.


tajo-core/src/main/java/org/apache/tajo/worker/Task.java
<https://reviews.apache.org/r/25242/#comment92600>

    It is trivial. Could you change it to a getter method for consitentcy?


- Hyunsik Choi


On Sept. 4, 2014, 5:40 p.m., Hai Thanh Mai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25242/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2014, 5:40 p.m.)
> 
> 
> Review request for Tajo and Hyunsik Choi.
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> TAJO-983: Worker should directly read Intermediate data stored in localhost 
> rather than fetching
> 
> Currently, worker always fetches all intermediate via Fetcher and than store 
> them in local file system even though some intermediate data already are 
> stored in local file system. It is inefficient and causes unnecessary I/O and 
> extra storage occupation. We should improve it.
> 
> https://issues.apache.org/jira/browse/TAJO-983
> 
> 
> Diffs
> -----
> 
>   tajo-core/src/main/java/org/apache/tajo/worker/Fetcher.java 64475fe 
>   tajo-core/src/main/java/org/apache/tajo/worker/Task.java d0665ae 
>   tajo-core/src/test/java/org/apache/tajo/worker/TestFetcher.java 95c06bb 
>   
> tajo-yarn-pullserver/src/main/java/org/apache/tajo/pullserver/TajoPullServerService.java
>  150ac85 
>   
> tajo-yarn-pullserver/src/main/java/org/apache/tajo/pullserver/retriever/FileChunk.java
>  a8b424e 
> 
> Diff: https://reviews.apache.org/r/25242/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hai Thanh Mai
> 
>

Reply via email to