Re: Review Request 66353: Connection resource related issues in DBOutputFormat and OracleManager
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66353/ --- (Updated April 6, 2018, 1:26 p.m.) Review request for Sqoop. Changes --- test case for oracle manager formatting Bugs: SQOOP-3306 https://issues.apache.org/jira/browse/SQOOP-3306 Repository: sqoop-trunk Description --- A fortify scan showed 2 possible resource leaks. 1: The function getRecordWriter() in DBOutputFormat.java sometimes fails to release a database resource allocated by getConnection() on line 117. In the file DBOutputFormat.java similar issues were on line numbers 117 2: The function makeConnection() in OracleManager.java sometimes fails to release a database resource allocated by getConnection() on line 321. In the file OracleManager.java similar issues were on line numbers 321 Diffs (updated) - src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java 730ff286 src/test/org/apache/sqoop/manager/TestOracleManager.java PRE-CREATION Diff: https://reviews.apache.org/r/66353/diff/2/ Changes: https://reviews.apache.org/r/66353/diff/1-2/ Testing --- ant test Thanks, Laszlo Bodor
Re: Review Request 66353: Connection resource related issues in DBOutputFormat and OracleManager
> On March 29, 2018, 9:32 a.m., Fero Szabo wrote: > > src/java/org/apache/sqoop/manager/OracleManager.java > > Lines 334-336 (patched) > > <https://reviews.apache.org/r/66353/diff/1/?file=1989923#file1989923line334> > > > > You probably missed some spaces here :) > > Fero Szabo wrote: > oops, the patch itself doesn't apply. (with git apply) Only with patch > -p0... > What command did you use to generate it? I've created the diff with --no-prefix, it's not suitable in this project, next time I'll generate with prefixes. - Laszlo --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66353/#review200179 ------- On March 29, 2018, 9:10 a.m., Laszlo Bodor wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66353/ > --- > > (Updated March 29, 2018, 9:10 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-3306 > https://issues.apache.org/jira/browse/SQOOP-3306 > > > Repository: sqoop-trunk > > > Description > --- > > A fortify scan showed 2 possible resource leaks. > > 1: The function getRecordWriter() in DBOutputFormat.java sometimes fails to > release a database resource allocated by getConnection() on line 117. > In the file DBOutputFormat.java similar issues were on line numbers 117 > > 2: The function makeConnection() in OracleManager.java sometimes fails to > release a database resource allocated by getConnection() on line 321. > In the file OracleManager.java similar issues were on line numbers 321 > > > Diffs > - > > src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 > src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java 730ff286 > > > Diff: https://reviews.apache.org/r/66353/diff/1/ > > > Testing > --- > > ant test > > > Thanks, > > Laszlo Bodor > >
Re: Review Request 66353: Connection resource related issues in DBOutputFormat and OracleManager
> On April 3, 2018, 9:13 a.m., Szabolcs Vasas wrote: > > Hi László, > > > > Thank you for submitting this improvement, it is a nice catch! > > It looks good to me however it would be great if you could create a couple > > of unit test cases to cover the code you have added. > > > > Thanks, > > Szabolcs I'll take care of it. - Laszlo --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66353/#review200344 ------- On March 29, 2018, 9:10 a.m., Laszlo Bodor wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66353/ > --- > > (Updated March 29, 2018, 9:10 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-3306 > https://issues.apache.org/jira/browse/SQOOP-3306 > > > Repository: sqoop-trunk > > > Description > --- > > A fortify scan showed 2 possible resource leaks. > > 1: The function getRecordWriter() in DBOutputFormat.java sometimes fails to > release a database resource allocated by getConnection() on line 117. > In the file DBOutputFormat.java similar issues were on line numbers 117 > > 2: The function makeConnection() in OracleManager.java sometimes fails to > release a database resource allocated by getConnection() on line 321. > In the file OracleManager.java similar issues were on line numbers 321 > > > Diffs > - > > src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 > src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java 730ff286 > > > Diff: https://reviews.apache.org/r/66353/diff/1/ > > > Testing > --- > > ant test > > > Thanks, > > Laszlo Bodor > >
Review Request 66353: Connection resource related issues in DBOutputFormat and OracleManager
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66353/ --- Review request for Sqoop. Bugs: SQOOP-3306 https://issues.apache.org/jira/browse/SQOOP-3306 Repository: sqoop-trunk Description --- A fortify scan showed 2 possible resource leaks. 1: The function getRecordWriter() in DBOutputFormat.java sometimes fails to release a database resource allocated by getConnection() on line 117. In the file DBOutputFormat.java similar issues were on line numbers 117 2: The function makeConnection() in OracleManager.java sometimes fails to release a database resource allocated by getConnection() on line 321. In the file OracleManager.java similar issues were on line numbers 321 Diffs - src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java 730ff286 Diff: https://reviews.apache.org/r/66353/diff/1/ Testing --- ant test Thanks, Laszlo Bodor
Re: Review Request 66282: Mock ConnManager field in TestTableDefWriter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66282/#review200178 --- Nice and clean refactor +1 - Laszlo Bodor On March 27, 2018, noon, Szabolcs Vasas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66282/ > --- > > (Updated March 27, 2018, noon) > > > Review request for Sqoop. > > > Bugs: SQOOP-3308 > https://issues.apache.org/jira/browse/SQOOP-3308 > > > Repository: sqoop-trunk > > > Description > --- > > This patch removes the externalColTypes field from TableDefWriter since it > was only used for testing purposes. > TestTableDefWriter is fixed to mock the ConnManager object provided to the > TableDefWriter constructor and a minor refactoring is done on the class. > > > Diffs > - > > src/java/org/apache/sqoop/hive/TableDefWriter.java e1424c383 > src/test/org/apache/sqoop/hive/TestTableDefWriter.java 496b5add9 > > > Diff: https://reviews.apache.org/r/66282/diff/4/ > > > Testing > --- > > ant clean test > > > Thanks, > > Szabolcs Vasas > >
[jira] [Commented] (SQOOP-3306) Connection resource related issues in DBOutputFormat and OracleManager
[
https://issues.apache.org/jira/browse/SQOOP-3306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16418616#comment-16418616
]
Laszlo Bodor commented on SQOOP-3306:
-
[~BoglarkaEgyed] : could you please help assign this ticket to me?
> Connection resource related issues in DBOutputFormat and OracleManager
> --
>
> Key: SQOOP-3306
> URL: https://issues.apache.org/jira/browse/SQOOP-3306
> Project: Sqoop
> Issue Type: Bug
>Affects Versions: 1.4.7
> Reporter: Laszlo Bodor
>Priority: Minor
> Fix For: 1.5.0
>
> Attachments: SQOOP-3306.01.patch
>
>
> A fortify scan showed 2 possible resource leaks.
> *Overview* : The function getRecordWriter() in DBOutputFormat.java sometimes
> fails to release a database resource allocated by getConnection() on line 117.
> In the file DBOutputFormat.java similar issues were on line numbers 117
>
> Connection should be closed before throwing IOException forward.
> {code:java}
> try {
> Connection connection = dbConf.getConnection();
> PreparedStatement statement = null;
> statement = connection.prepareStatement(
> constructQuery(tableName, fieldNames));
> return new org.apache.sqoop.mapreduce.db.DBOutputFormat.DBRecordWriter(
> connection, statement);
> } catch (Exception ex) {
> throw new IOException(ex);
> }
> {code}
>
> *Overview* : The function makeConnection() in OracleManager.java sometimes
> fails to release a database resource allocated by getConnection() on line 321.
> In the file OracleManager.java similar issues were on line numbers 321
>
> Some connection post setup steps could be extracted to a separate method
> which could be enclosed by a try/catch.
> {code:java}
> connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
>
> setSessionTimeZone(connection);
> connection.setAutoCommit(false);
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (SQOOP-3306) Connection resource related issues in DBOutputFormat and OracleManager
[
https://issues.apache.org/jira/browse/SQOOP-3306?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laszlo Bodor updated SQOOP-3306:
Attachment: SQOOP-3306.01.patch
> Connection resource related issues in DBOutputFormat and OracleManager
> --
>
> Key: SQOOP-3306
> URL: https://issues.apache.org/jira/browse/SQOOP-3306
> Project: Sqoop
> Issue Type: Bug
>Affects Versions: 1.4.7
> Reporter: Laszlo Bodor
>Priority: Minor
> Fix For: 1.5.0
>
> Attachments: SQOOP-3306.01.patch
>
>
> A fortify scan showed 2 possible resource leaks.
> *Overview* : The function getRecordWriter() in DBOutputFormat.java sometimes
> fails to release a database resource allocated by getConnection() on line 117.
> In the file DBOutputFormat.java similar issues were on line numbers 117
>
> Connection should be closed before throwing IOException forward.
> {code:java}
> try {
> Connection connection = dbConf.getConnection();
> PreparedStatement statement = null;
> statement = connection.prepareStatement(
> constructQuery(tableName, fieldNames));
> return new org.apache.sqoop.mapreduce.db.DBOutputFormat.DBRecordWriter(
> connection, statement);
> } catch (Exception ex) {
> throw new IOException(ex);
> }
> {code}
>
> *Overview* : The function makeConnection() in OracleManager.java sometimes
> fails to release a database resource allocated by getConnection() on line 321.
> In the file OracleManager.java similar issues were on line numbers 321
>
> Some connection post setup steps could be extracted to a separate method
> which could be enclosed by a try/catch.
> {code:java}
> connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
>
> setSessionTimeZone(connection);
> connection.setAutoCommit(false);
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (SQOOP-3306) Connection resource related issues in DBOutputFormat and OracleManager
[
https://issues.apache.org/jira/browse/SQOOP-3306?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laszlo Bodor updated SQOOP-3306:
Description:
A fortify scan showed 2 possible resource leaks.
*Overview* : The function getRecordWriter() in DBOutputFormat.java sometimes
fails to release a database resource allocated by getConnection() on line 117.
In the file DBOutputFormat.java similar issues were on line numbers 117
Connection should be closed before throwing IOException forward.
{code:java}
try {
Connection connection = dbConf.getConnection();
PreparedStatement statement = null;
statement = connection.prepareStatement(
constructQuery(tableName, fieldNames));
return new org.apache.sqoop.mapreduce.db.DBOutputFormat.DBRecordWriter(
connection, statement);
} catch (Exception ex) {
throw new IOException(ex);
}
{code}
*Overview* : The function makeConnection() in OracleManager.java sometimes
fails to release a database resource allocated by getConnection() on line 321.
In the file OracleManager.java similar issues were on line numbers 321
Some connection post setup steps could be extracted to a separate method which
could be enclosed by a try/catch.
{code:java}
connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
setSessionTimeZone(connection);
connection.setAutoCommit(false);
{code}
was:
A fortify scan showed 2 possible resource leaks.
*Overview* : The function getRecordWriter() in DBOutputFormat.java sometimes
fails to release a database resource allocated by getConnection() on line 117.
In the file DBOutputFormat.java similar issues were on line numbers 117
Connection should be closed before throwing IOException forward.
{code}
try {
Connection connection = dbConf.getConnection();
PreparedStatement statement = null;
statement = connection.prepareStatement(
constructQuery(tableName, fieldNames));
return new org.apache.sqoop.mapreduce.db.DBOutputFormat.DBRecordWriter(
connection, statement);
} catch (Exception ex) {
throw new IOException(ex);
}
{code}
*Overview* : The function makeConnection() in OracleManager.java sometimes
fails to release a database resource allocated by getConnection() on line 321.
In the file OracleManager.java similar issues were on line numbers 321
Some connection setup steps could extract to a separated method which could be
enclosed by a try/catch.
{code}
connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
setSessionTimeZone(connection);
connection.setAutoCommit(false);
{code}
> Connection resource related issues in DBOutputFormat and OracleManager
> --
>
> Key: SQOOP-3306
> URL: https://issues.apache.org/jira/browse/SQOOP-3306
> Project: Sqoop
> Issue Type: Bug
>Affects Versions: 1.4.7
> Reporter: Laszlo Bodor
>Priority: Minor
> Fix For: 1.5.0
>
>
> A fortify scan showed 2 possible resource leaks.
> *Overview* : The function getRecordWriter() in DBOutputFormat.java sometimes
> fails to release a database resource allocated by getConnection() on line 117.
> In the file DBOutputFormat.java similar issues were on line numbers 117
>
> Connection should be closed before throwing IOException forward.
> {code:java}
> try {
> Connection connection = dbConf.getConnection();
> PreparedStatement statement = null;
> statement = connection.prepareStatement(
> constructQuery(tableName, fieldNames));
> return new org.apache.sqoop.mapreduce.db.DBOutputFormat.DBRecordWriter(
> connection, statement);
> } catch (Exception ex) {
> throw new IOException(ex);
> }
> {code}
>
> *Overview* : The function makeConnection() in OracleManager.java sometimes
> fails to release a database resource allocated by getConnection() on line 321.
> In the file OracleManager.java similar issues were on line numbers 321
>
> Some connection post setup steps could be extracted to a separate method
> which could be enclosed by a try/catch.
> {code:java}
> connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
>
> setSessionTimeZone(connection);
> connection.setAutoCommit(false);
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (SQOOP-3306) Connection resource related issues in DBOutputFormat and OracleManager
Laszlo Bodor created SQOOP-3306:
---
Summary: Connection resource related issues in DBOutputFormat and
OracleManager
Key: SQOOP-3306
URL: https://issues.apache.org/jira/browse/SQOOP-3306
Project: Sqoop
Issue Type: Bug
Affects Versions: 1.4.7
Reporter: Laszlo Bodor
Fix For: 1.5.0
A fortify scan showed 2 possible resource leaks.
*Overview* : The function getRecordWriter() in DBOutputFormat.java sometimes
fails to release a database resource allocated by getConnection() on line 117.
In the file DBOutputFormat.java similar issues were on line numbers 117
Connection should be closed before throwing IOException forward.
{code}
try {
Connection connection = dbConf.getConnection();
PreparedStatement statement = null;
statement = connection.prepareStatement(
constructQuery(tableName, fieldNames));
return new org.apache.sqoop.mapreduce.db.DBOutputFormat.DBRecordWriter(
connection, statement);
} catch (Exception ex) {
throw new IOException(ex);
}
{code}
*Overview* : The function makeConnection() in OracleManager.java sometimes
fails to release a database resource allocated by getConnection() on line 321.
In the file OracleManager.java similar issues were on line numbers 321
Some connection setup steps could extract to a separated method which could be
enclosed by a try/catch.
{code}
connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
setSessionTimeZone(connection);
connection.setAutoCommit(false);
{code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
