Re: Review Request 68606: Error during direct Netezza import/export can interrupt process in uncontrolled ways

[Szabolcs Vasas]

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


Ship it!




Ship It!

- Szabolcs Vasas


On Sept. 3, 2018, 11:32 a.m., daniel voros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68606/
> ---
> 
> (Updated Sept. 3, 2018, 11:32 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3378
> https://issues.apache.org/jira/browse/SQOOP-3378
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> `SQLException` during JDBC operation in direct Netezza import/export signals 
> parent thread to fail fast by interrupting it.
> We're trying to process the interrupt in the parent (main) thread, but 
> there's no guarantee that we're not in some internal call that will process 
> the interrupted flag and reset it before we're able to check.
> 
> It is also possible that the parent thread has passed the "checking part" 
> when it gets interrupted. In case of `NetezzaExternalTableExportMapper` this 
> can interrupt the upload of log files.
> 
> I'd recommend using some other means of communication between the threads 
> than interrupts.
> 
> 
> Diffs
> -
> 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
>  5bf21880 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
>  306062aa 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java
>  cedfd235 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableExportMapper.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableImportMapper.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68606/diff/2/
> 
> 
> Testing
> ---
> 
> added new UTs and checked manual Netezza tests (NetezzaExportManualTest, 
> NetezzaImportManualTest)
> 
> 
> Thanks,
> 
> daniel voros
> 
>

Re: Review Request 68606: Error during direct Netezza import/export can interrupt process in uncontrolled ways

[Boglarka Egyed]

> On Oct. 4, 2018, 12:46 p.m., Boglarka Egyed wrote:
> > Hi Daniel,
> > 
> > Apart from the discussion with Szabolcs about the expected exception 
> > handling I'm OK with your change. All tests passed.
> > 
> > Thanks,
> > Bogi
> 
> daniel voros wrote:
> Hey Bogi,
> 
> Thanks for reviewing! What do you mean by expected exception handling? 
> I'm happy to update the patch if you have concerns!
> 
> Regards,
> Daniel

Hi Daniel,

I apologize, I confused your patch with another one, sorry about that. Please 
ignore my previous comment regarding expected exceptions.
Ship it! :)

Regards,
Bogi


- Boglarka


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


On Sept. 3, 2018, 11:32 a.m., daniel voros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68606/
> ---
> 
> (Updated Sept. 3, 2018, 11:32 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3378
> https://issues.apache.org/jira/browse/SQOOP-3378
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> `SQLException` during JDBC operation in direct Netezza import/export signals 
> parent thread to fail fast by interrupting it.
> We're trying to process the interrupt in the parent (main) thread, but 
> there's no guarantee that we're not in some internal call that will process 
> the interrupted flag and reset it before we're able to check.
> 
> It is also possible that the parent thread has passed the "checking part" 
> when it gets interrupted. In case of `NetezzaExternalTableExportMapper` this 
> can interrupt the upload of log files.
> 
> I'd recommend using some other means of communication between the threads 
> than interrupts.
> 
> 
> Diffs
> -
> 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
>  5bf21880 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
>  306062aa 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java
>  cedfd235 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableExportMapper.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableImportMapper.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68606/diff/2/
> 
> 
> Testing
> ---
> 
> added new UTs and checked manual Netezza tests (NetezzaExportManualTest, 
> NetezzaImportManualTest)
> 
> 
> Thanks,
> 
> daniel voros
> 
>

Re: Review Request 68606: Error during direct Netezza import/export can interrupt process in uncontrolled ways

[Boglarka Egyed]

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


Ship it!




Hi Daniel,

Apart from the discussion with Szabolcs about the expected exception handling 
I'm OK with your change. All tests passed.

Thanks,
Bogi

- Boglarka Egyed


On Sept. 3, 2018, 11:32 a.m., daniel voros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68606/
> ---
> 
> (Updated Sept. 3, 2018, 11:32 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3378
> https://issues.apache.org/jira/browse/SQOOP-3378
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> `SQLException` during JDBC operation in direct Netezza import/export signals 
> parent thread to fail fast by interrupting it.
> We're trying to process the interrupt in the parent (main) thread, but 
> there's no guarantee that we're not in some internal call that will process 
> the interrupted flag and reset it before we're able to check.
> 
> It is also possible that the parent thread has passed the "checking part" 
> when it gets interrupted. In case of `NetezzaExternalTableExportMapper` this 
> can interrupt the upload of log files.
> 
> I'd recommend using some other means of communication between the threads 
> than interrupts.
> 
> 
> Diffs
> -
> 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
>  5bf21880 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
>  306062aa 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java
>  cedfd235 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableExportMapper.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableImportMapper.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68606/diff/2/
> 
> 
> Testing
> ---
> 
> added new UTs and checked manual Netezza tests (NetezzaExportManualTest, 
> NetezzaImportManualTest)
> 
> 
> Thanks,
> 
> daniel voros
> 
>

Re: Review Request 68606: Error during direct Netezza import/export can interrupt process in uncontrolled ways

[Szabolcs Vasas]

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



Hi Daniel,

Thank you for submitting this patch, I think it is a good idea to use 
AtomicBoolean instead of the interrupted flag.
I have left a small question only.
The unit and third party tests are running fine.


src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
Line 229 (original)


The isAlive check is not removed in NetezzaExternalTableImportMapper, is 
there a particular reason for that?


- Szabolcs Vasas


On Sept. 3, 2018, 11:32 a.m., daniel voros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68606/
> ---
> 
> (Updated Sept. 3, 2018, 11:32 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3378
> https://issues.apache.org/jira/browse/SQOOP-3378
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> `SQLException` during JDBC operation in direct Netezza import/export signals 
> parent thread to fail fast by interrupting it.
> We're trying to process the interrupt in the parent (main) thread, but 
> there's no guarantee that we're not in some internal call that will process 
> the interrupted flag and reset it before we're able to check.
> 
> It is also possible that the parent thread has passed the "checking part" 
> when it gets interrupted. In case of `NetezzaExternalTableExportMapper` this 
> can interrupt the upload of log files.
> 
> I'd recommend using some other means of communication between the threads 
> than interrupts.
> 
> 
> Diffs
> -
> 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
>  5bf21880 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
>  306062aa 
>   
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java
>  cedfd235 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableExportMapper.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableImportMapper.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68606/diff/1/
> 
> 
> Testing
> ---
> 
> added new UTs and checked manual Netezza tests (NetezzaExportManualTest, 
> NetezzaImportManualTest)
> 
> 
> Thanks,
> 
> daniel voros
> 
>

Review Request 68606: Error during direct Netezza import/export can interrupt process in uncontrolled ways

[daniel voros]

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

Review request for Sqoop.


Bugs: SQOOP-3378
https://issues.apache.org/jira/browse/SQOOP-3378


Repository: sqoop-trunk


Description
---

`SQLException` during JDBC operation in direct Netezza import/export signals 
parent thread to fail fast by interrupting it.
We're trying to process the interrupt in the parent (main) thread, but there's 
no guarantee that we're not in some internal call that will process the 
interrupted flag and reset it before we're able to check.

It is also possible that the parent thread has passed the "checking part" when 
it gets interrupted. In case of `NetezzaExternalTableExportMapper` this can 
interrupt the upload of log files.

I'd recommend using some other means of communication between the threads than 
interrupts.


Diffs
-

  
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
 5bf21880 
  
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
 306062aa 
  
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java 
cedfd235 
  
src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableExportMapper.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/mapreduce/db/netezza/TestNetezzaExternalTableImportMapper.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/68606/diff/1/


Testing
---

added new UTs and checked manual Netezza tests (NetezzaExportManualTest, 
NetezzaImportManualTest)


Thanks,

daniel voros