> 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?
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. - Mengwei ----------------------------------------------------------- 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 > >
