Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop >> mail # dev >> Review Request: SQOOP-863 Sqoop2: Introduce ProgressThread into Extractor and Loader


Copy link to this message
-
Re: Review Request: SQOOP-863 Sqoop2: Introduce ProgressThread into Extractor and Loader

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9845/#review18399
-----------------------------------------------------------
Thanks for the patch, Jarcec! I have some very minor feedback below, otherwise I think this patch is good to go.
execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/ProgressRunnable.java
<https://reviews.apache.org/r/9845/#comment38594>

    This can be made final.

execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java
<https://reviews.apache.org/r/9845/#comment38595>

    This can be made final.

execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java
<https://reviews.apache.org/r/9845/#comment38597>

    The general idiom is to wait for sometime after shutdown and then call shutdownNow() to ensure the executor will be shutdown eventually by interrupting the threads (the shutdown method only makes sure the threads are not scheduled again, it does not interrupt currently running threads).
    
    So you should probably change this to:
    progressService.shutdown();
    if(!progressService.awaitTermination(5, TimeUnit.SECONDS) {
    progressService.shutdownNow();
    }

execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopReducer.java
<https://reviews.apache.org/r/9845/#comment38596>

    This can be made final.
- Hari Shreedharan
On March 24, 2013, 10:56 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9845/
> -----------------------------------------------------------
>
> (Updated March 24, 2013, 10:56 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've ported ProgressThread from Sqoop1 and plug it into SqoopMapper and SqoopReducer.
>
>
> This addresses bug SQOOP-863.
>     https://issues.apache.org/jira/browse/SQOOP-863
>
>
> Diffs
> -----
>
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/ProgressRunnable.java PRE-CREATION
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java 2a823032cfd91fb8fd1a8aee3ffd9d80c2e3bae6
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopReducer.java d2361482771eea1a34b14c767952c5592f89c45a
>
> Diff: https://reviews.apache.org/r/9845/diff/
>
>
> Testing
> -------
>
> Unit and integration tests seems to be passing. I've also verified the functionality on real cluster.
>
>
> Thanks,
>
> Jarek Cecho
>
>