> On June 19, 2013, 11:39 p.m., Jarek Cecho wrote: > > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryProvider.java, > > lines 269-275 > > <https://reviews.apache.org/r/11920/diff/3/?file=308232#file308232line269> > > > > I do not feel comfortable about swallowing IllegalStateException. Do we > > have a reason why to catch it? Would it be simpler to let it propagate and > > kill the server? Anyway Sqoop server can't work without the repository > > right? > > Mengwei Ding wrote: > I checked through the source code of PoolableConnectionFactory. In the > construction method, it use connectionPool.setFactory() method to connect the > connectionFactory and the connection pool. The documentation of this method > says that "Trying to change the factory while there are borrowed objects will > throw an IllegalStateException.". Actually, this is what I was concerned, > because it happens that there might be several connections, which are > borrowed from the connection pool, still in use. We could not update the > connection pool in this case. So I just catch this IllegalStateException to > inform the administrator whether it's ok to apply the new configuration > changes to the repository at this time.
Hi Mengwei, thank you very much for the explanation! It make sense to me now, please disregard my concern. Jarcec - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11920/#review22137 ----------------------------------------------------------- On June 19, 2013, 1:06 a.m., Mengwei Ding wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11920/ > ----------------------------------------------------------- > > (Updated June 19, 2013, 1:06 a.m.) > > > Review request for Sqoop, Jarek Cecho, Hari Shreedharan, and Abraham Elmahrek. > > > Description > ------- > > commit 7fc5ae741cbe74e3f9aa50138dadfc3fdb594cbd > Author: Mengwei Ding <mengwei.d...@cloudera.com> > Date: Wed Jun 12 15:47:26 2013 -0700 > > SQOOP-971 Add dynamic reconfiguration ability to RepositoryManager, > ConnectorManager and FrameworkManager > > :100644 100644 500189a... 0540f6b... M > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java > :100644 100644 f59d132... eb7c1dc... M > core/src/main/java/org/apache/sqoop/core/CoreError.java > :000000 100644 0000000... d25ce41... A > core/src/main/java/org/apache/sqoop/core/Reconfigurable.java > :100644 100644 deb24c9... c34ab37... M > core/src/main/java/org/apache/sqoop/core/SqoopConfiguration.java > :100644 100644 145a2c1... bfe0971... M > core/src/main/java/org/apache/sqoop/framework/FrameworkManager.java > :100644 100644 3339c59... 7f47326... M > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryProvider.java > :100644 100644 955306d... 88667f2... M > core/src/main/java/org/apache/sqoop/repository/RepositoryManager.java > :100644 100644 4ea52e9... 1ec6bdf... M > core/src/main/java/org/apache/sqoop/repository/RepositoryProvider.java > :100644 100644 de9a24b... ae8686b... M > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > > > This addresses bug SQOOP-971. > https://issues.apache.org/jira/browse/SQOOP-971 > > > Diffs > ----- > > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 500189a > core/src/main/java/org/apache/sqoop/core/CoreError.java f59d132 > core/src/main/java/org/apache/sqoop/core/Reconfigurable.java PRE-CREATION > core/src/main/java/org/apache/sqoop/core/SqoopConfiguration.java deb24c9 > core/src/main/java/org/apache/sqoop/framework/FrameworkManager.java 145a2c1 > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryProvider.java > 3339c59 > core/src/main/java/org/apache/sqoop/repository/RepositoryManager.java > 955306d > core/src/main/java/org/apache/sqoop/repository/RepositoryProvider.java > 4ea52e9 > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > de9a24b > > Diff: https://reviews.apache.org/r/11920/diff/ > > > Testing > ------- > > > Thanks, > > Mengwei Ding > >
