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

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


+
Jarek Cecho 2013-03-11, 02:53
+
Hari Shreedharan 2013-03-11, 06:13
+
Jarek Cecho 2013-03-24, 22:57
+
Jarek Cecho 2013-03-24, 22:56
+
Hari Shreedharan 2013-03-26, 16:53
+
Jarek Cecho 2013-03-27, 00:40
Copy link to this message
-
Re: Review Request: SQOOP-863 Sqoop2: Introduce ProgressThread into Extractor and Loader
Hari Shreedharan 2013-03-29, 18:02

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9845/#review18521
-----------------------------------------------------------
Hi Jarcec,

This patch looks great. This is committable as-is. The only question I have is that the behavior of the original patch (rev 1) and this one have changed in the following ways:

1. The time between the progress calls was configurable in the first patch, but not this one (came from the context instance I guess - in the first patch)
2. The thread was daemon in rev 1, but it is no longer (you can pass in a thread factory to the newScheduledSingleThreadedExecutor call and make your thread daemon before returning - you can also use Guava's ThreadFactoryBuilder if you want). By passing in a thread factory or using ThreadFactoryBuilder, you can name the thread too.
If these two are not much of a concern, +1 from me.

- Hari Shreedharan
On March 27, 2013, 12:40 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9845/
> -----------------------------------------------------------
>
> (Updated March 27, 2013, 12:40 a.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
>
>

+
Jarek Cecho 2013-03-31, 04:38
+
Hari Shreedharan 2013-03-31, 08:29
+
Hari Shreedharan 2013-03-31, 08:28
+
Kathleen Ting 2013-04-07, 05:18