You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by vinodkc <gi...@git.apache.org> on 2017/08/06 09:39:41 UTC
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
GitHub user vinodkc opened a pull request:
https://github.com/apache/storm/pull/2262
STORM-2679: Need to close resources and streams after use
Resources in JdbcClient.java,FluxParser.java,AutoSSL.java classes are released after use.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vinodkc/storm-1 br_fixresourceLeak
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2262.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2262
----
commit 0cf4d7766f6ff9a42a7625b6c1dec93cc535cc7e
Author: vinodkc <vi...@gmail.com>
Date: 2017-08-06T09:25:10Z
release resources
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2262#discussion_r131788473
--- Diff: flux/flux-core/src/main/java/org/apache/storm/flux/parser/FluxParser.java ---
@@ -96,11 +96,12 @@ private static TopologyDef loadYaml(Yaml yaml, InputStream in, String propsFile,
// properties file substitution
if(propsFile != null){
LOG.info("Performing property substitution.");
- InputStream propsIn = new FileInputStream(propsFile);
- Properties props = new Properties();
- props.load(propsIn);
- for(Object key : props.keySet()){
- str = str.replace("${" + key + "}", props.getProperty((String)key));
+ try(InputStream propsIn = new FileInputStream(propsFile)) {
--- End diff --
space between `try` and `(`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2262: STORM-2679: Need to close resources and streams after use
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2262
Post +1 (missed)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2262
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2262#discussion_r131787609
--- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java ---
@@ -106,50 +107,52 @@ public String apply(Column input) {
Connection connection = null;
try {
connection = connectionProvider.getConnection();
- PreparedStatement preparedStatement = connection.prepareStatement(sqlQuery);
- if(queryTimeoutSecs > 0) {
- preparedStatement.setQueryTimeout(queryTimeoutSecs);
- }
- setPreparedStatementParams(preparedStatement, queryParams);
- ResultSet resultSet = preparedStatement.executeQuery();
- List<List<Column>> rows = Lists.newArrayList();
- while(resultSet.next()){
- ResultSetMetaData metaData = resultSet.getMetaData();
- int columnCount = metaData.getColumnCount();
- List<Column> row = Lists.newArrayList();
- for(int i=1 ; i <= columnCount; i++) {
- String columnLabel = metaData.getColumnLabel(i);
- int columnType = metaData.getColumnType(i);
- Class columnJavaType = Util.getJavaType(columnType);
- if (columnJavaType.equals(String.class)) {
- row.add(new Column<String>(columnLabel, resultSet.getString(columnLabel), columnType));
- } else if (columnJavaType.equals(Integer.class)) {
- row.add(new Column<Integer>(columnLabel, resultSet.getInt(columnLabel), columnType));
- } else if (columnJavaType.equals(Double.class)) {
- row.add(new Column<Double>(columnLabel, resultSet.getDouble(columnLabel), columnType));
- } else if (columnJavaType.equals(Float.class)) {
- row.add(new Column<Float>(columnLabel, resultSet.getFloat(columnLabel), columnType));
- } else if (columnJavaType.equals(Short.class)) {
- row.add(new Column<Short>(columnLabel, resultSet.getShort(columnLabel), columnType));
- } else if (columnJavaType.equals(Boolean.class)) {
- row.add(new Column<Boolean>(columnLabel, resultSet.getBoolean(columnLabel), columnType));
- } else if (columnJavaType.equals(byte[].class)) {
- row.add(new Column<byte[]>(columnLabel, resultSet.getBytes(columnLabel), columnType));
- } else if (columnJavaType.equals(Long.class)) {
- row.add(new Column<Long>(columnLabel, resultSet.getLong(columnLabel), columnType));
- } else if (columnJavaType.equals(Date.class)) {
- row.add(new Column<Date>(columnLabel, resultSet.getDate(columnLabel), columnType));
- } else if (columnJavaType.equals(Time.class)) {
- row.add(new Column<Time>(columnLabel, resultSet.getTime(columnLabel), columnType));
- } else if (columnJavaType.equals(Timestamp.class)) {
- row.add(new Column<Timestamp>(columnLabel, resultSet.getTimestamp(columnLabel), columnType));
- } else {
- throw new RuntimeException("type = " + columnType + " for column " + columnLabel + " not supported.");
+ try(PreparedStatement preparedStatement = connection.prepareStatement(sqlQuery)) {
+ if (queryTimeoutSecs > 0) {
+ preparedStatement.setQueryTimeout(queryTimeoutSecs);
+ }
+ setPreparedStatementParams(preparedStatement, queryParams);
+ try(ResultSet resultSet = preparedStatement.executeQuery()) {
--- End diff --
space between `try` and `(`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2262#discussion_r131787582
--- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java ---
@@ -106,50 +107,52 @@ public String apply(Column input) {
Connection connection = null;
try {
connection = connectionProvider.getConnection();
- PreparedStatement preparedStatement = connection.prepareStatement(sqlQuery);
- if(queryTimeoutSecs > 0) {
- preparedStatement.setQueryTimeout(queryTimeoutSecs);
- }
- setPreparedStatementParams(preparedStatement, queryParams);
- ResultSet resultSet = preparedStatement.executeQuery();
- List<List<Column>> rows = Lists.newArrayList();
- while(resultSet.next()){
- ResultSetMetaData metaData = resultSet.getMetaData();
- int columnCount = metaData.getColumnCount();
- List<Column> row = Lists.newArrayList();
- for(int i=1 ; i <= columnCount; i++) {
- String columnLabel = metaData.getColumnLabel(i);
- int columnType = metaData.getColumnType(i);
- Class columnJavaType = Util.getJavaType(columnType);
- if (columnJavaType.equals(String.class)) {
- row.add(new Column<String>(columnLabel, resultSet.getString(columnLabel), columnType));
- } else if (columnJavaType.equals(Integer.class)) {
- row.add(new Column<Integer>(columnLabel, resultSet.getInt(columnLabel), columnType));
- } else if (columnJavaType.equals(Double.class)) {
- row.add(new Column<Double>(columnLabel, resultSet.getDouble(columnLabel), columnType));
- } else if (columnJavaType.equals(Float.class)) {
- row.add(new Column<Float>(columnLabel, resultSet.getFloat(columnLabel), columnType));
- } else if (columnJavaType.equals(Short.class)) {
- row.add(new Column<Short>(columnLabel, resultSet.getShort(columnLabel), columnType));
- } else if (columnJavaType.equals(Boolean.class)) {
- row.add(new Column<Boolean>(columnLabel, resultSet.getBoolean(columnLabel), columnType));
- } else if (columnJavaType.equals(byte[].class)) {
- row.add(new Column<byte[]>(columnLabel, resultSet.getBytes(columnLabel), columnType));
- } else if (columnJavaType.equals(Long.class)) {
- row.add(new Column<Long>(columnLabel, resultSet.getLong(columnLabel), columnType));
- } else if (columnJavaType.equals(Date.class)) {
- row.add(new Column<Date>(columnLabel, resultSet.getDate(columnLabel), columnType));
- } else if (columnJavaType.equals(Time.class)) {
- row.add(new Column<Time>(columnLabel, resultSet.getTime(columnLabel), columnType));
- } else if (columnJavaType.equals(Timestamp.class)) {
- row.add(new Column<Timestamp>(columnLabel, resultSet.getTimestamp(columnLabel), columnType));
- } else {
- throw new RuntimeException("type = " + columnType + " for column " + columnLabel + " not supported.");
+ try(PreparedStatement preparedStatement = connection.prepareStatement(sqlQuery)) {
--- End diff --
space between `try` and `(`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2262#discussion_r131789023
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/AutoSSL.java ---
@@ -133,8 +134,9 @@ public static void deserializeSSLFile(String credsKey, String directory,
if (resultStr != null) {
byte[] decodedData = DatatypeConverter.parseBase64Binary(resultStr);
File f = new File(directory, credsKey);
- FileOutputStream fout = new FileOutputStream(f);
- fout.write(decodedData);
+ try(FileOutputStream fout = new FileOutputStream(f)) {
--- End diff --
space between `try` and `(`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2262: STORM-2679: Need to close resources and streams after use
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2262
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2262#discussion_r131788092
--- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java ---
@@ -179,8 +182,9 @@ public void executeSql(String sql) {
Connection connection = null;
try {
connection = connectionProvider.getConnection();
- Statement statement = connection.createStatement();
- statement.execute(sql);
+ try(Statement statement = connection.createStatement()) {
--- End diff --
space between `try` and `(`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2262#discussion_r131787411
--- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java ---
@@ -56,25 +56,26 @@ public void executeInsertQuery(String query, List<List<Column>> columnLists) {
LOG.debug("Executing query {}", query);
- PreparedStatement preparedStatement = connection.prepareStatement(query);
- if(queryTimeoutSecs > 0) {
- preparedStatement.setQueryTimeout(queryTimeoutSecs);
- }
+ try(PreparedStatement preparedStatement = connection.prepareStatement(query)) {
--- End diff --
space between `try` and `(`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2262: STORM-2679: Need to close resources and streams af...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2262#discussion_r131788989
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/AutoSSL.java ---
@@ -100,19 +100,20 @@ public void populateCredentials(Map<String, String> credentials) {
// the key.
public static void serializeSSLFile(String readFile, Map<String, String> credentials) {
try {
- FileInputStream in = new FileInputStream(readFile);
- LOG.debug("serializing ssl file: {}", readFile);
- ByteArrayOutputStream result = new ByteArrayOutputStream();
- byte[] buffer = new byte[4096];
- int length;
- while ((length = in.read(buffer)) != -1) {
- result.write(buffer, 0, length);
- }
- String resultStr = DatatypeConverter.printBase64Binary(result.toByteArray());
+ try(FileInputStream in = new FileInputStream(readFile)) {
--- End diff --
space between `try` and `(`, and let's not make it unnecessary nested. It can be flattened.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---