[ https://issues.apache.org/jira/browse/BEAM-3500?focusedWorklogId=79520&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-79520 ]
ASF GitHub Bot logged work on BEAM-3500: ---------------------------------------- Author: ASF GitHub Bot Created on: 12/Mar/18 16:48 Start Date: 12/Mar/18 16:48 Worklog Time Spent: 10m Work Description: jkff commented on a change in pull request #4461: [BEAM-3500] "Attach" JDBC connection to the bundle and add DataSourceFactory allowing full control of the way the DataSource is created URL: https://github.com/apache/beam/pull/4461#discussion_r173867345 ########## File path: sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcIO.java ########## @@ -525,18 +596,24 @@ public void populateDisplayData(DisplayData.Builder builder) { private ReadFn( DataSourceConfiguration dataSourceConfiguration, + DataSourceFactory dataSourceFactory, ValueProvider<String> query, PreparedStatementSetter<ParameterT> parameterSetter, RowMapper<OutputT> rowMapper) { this.dataSourceConfiguration = dataSourceConfiguration; + this.dataSourceFactory = dataSourceFactory; this.query = query; this.parameterSetter = parameterSetter; this.rowMapper = rowMapper; } @Setup public void setup() throws Exception { - dataSource = dataSourceConfiguration.buildDatasource(); + if (dataSourceFactory != null) { + dataSource = dataSourceFactory.create(); Review comment: In other words: In my view, dataSourceFactory is rather for cases where the DataSource requires some trickery to even construct, e.g. requires setting some vendor-specific connection parameters to establish the connection. I don't know if such cases exist at all, and I'd be open to simply dropping this feature from this PR. But if we think it's necessary, then I think we still want to wrap that with pooling, and I think for a user to try to configure pooling themselves is always a bad idea. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 79520) Time Spent: 4h (was: 3h 50m) > JdbcIO: Improve connection management > ------------------------------------- > > Key: BEAM-3500 > URL: https://issues.apache.org/jira/browse/BEAM-3500 > Project: Beam > Issue Type: Improvement > Components: io-java-jdbc > Affects Versions: 2.2.0 > Reporter: Pawel Bartoszek > Assignee: Jean-Baptiste Onofré > Priority: Major > Time Spent: 4h > Remaining Estimate: 0h > > JdbcIO write DoFn acquires connection in {{@Setup}} and release it in > {{@Teardown}} methods, which means that connection might stay opened for days > in streaming job case. Keeping single connection open for so long might be > very risky as it's exposed to database, network etc issues. > *Taking connection from the pool when it is actually needed* > I suggest that connection would be taken from the connection pool in > {{executeBatch}} method and released when the batch is flushed. This will > allow the pool to take care of any returned unhealthy connections etc. > *Make JdbcIO accept data source factory* > It would be nice if JdbcIO accepted DataSourceFactory rather than DataSource > itself. I am saying that because sink checks if DataSource implements > `Serializable` interface, which make it impossible to pass > BasicDataSource(used internally by sink) as it doesn’t implement this > interface. Something like: > {code:java} > interface DataSourceFactory extends Serializable{ > DataSource create(); > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)