Github user satishd commented on a diff in the pull request:
https://github.com/apache/storm/pull/2134#discussion_r118135735
--- Diff:
external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/AbstractJdbcBolt.java
---
@@ -42,26 +41,66 @@
protected Integer queryTimeoutSecs;
protected ConnectionProvider connectionProvider;
+ /**
+ * Subclasses should call this to ensure output collector and
connection
+ * provider are set up, and finally jdbcClient is initialized properly.
+ * <p/>
+ * {@inheritDoc}
+ */
@Override
- public void prepare(Map map, TopologyContext topologyContext,
OutputCollector collector) {
- this.collector = collector;
+ public void prepare(final Map map, final TopologyContext
topologyContext,
+ final OutputCollector outputCollector) {
+ AbstractJdbcBolt.avoidDeadlockFromDriverInitialization();
+
+ this.collector = outputCollector;
connectionProvider.prepare();
- if(queryTimeoutSecs == null) {
- queryTimeoutSecs =
Integer.parseInt(map.get(Config.TOPOLOGY_MESSAGE_TIMEOUT_SECS).toString());
+ if (queryTimeoutSecs == null) {
+ String msgTimeout =
map.get(Config.TOPOLOGY_MESSAGE_TIMEOUT_SECS)
+ .toString();
+ queryTimeoutSecs = Integer.parseInt(msgTimeout);
}
this.jdbcClient = new JdbcClient(connectionProvider,
queryTimeoutSecs);
}
- public AbstractJdbcBolt(ConnectionProvider connectionProvider) {
- Validate.notNull(connectionProvider);
- this.connectionProvider = connectionProvider;
+ /**
+ * Constructor.
+ * <p/>
+ * @param connectionProviderParam database connection provider
+ */
+ public AbstractJdbcBolt(final ConnectionProvider
connectionProviderParam) {
+ Validate.notNull(connectionProviderParam);
+ this.connectionProvider = connectionProviderParam;
}
+ /**
+ * Cleanup.
+ * <p/>
+ * Subclasses should call this to ensure connection provider can be
+ * also cleaned up.
+ */
@Override
public void cleanup() {
connectionProvider.cleanup();
}
+
+ /**
+ * Load DriverManager first to avoid any race condition between
+ * DriverManager static initialization block and specific driver
class's
+ * static initialization block. e.g. PhoenixDriver
+ * <p/>
+ * We should take this workaround since prepare() method is
synchronized
+ * but an worker can initialize multiple AbstractJdbcBolt instances and
+ * they would make race condition.
+ * <p/>
+ * We just need to ensure that DriverManager class is always
initialized
+ * earlier than provider so below line should be called first
+ * than initializing provider.
+ */
+ private static synchronized void
avoidDeadlockFromDriverInitialization() {
+ DriverManager.getDrivers();
--- End diff --
Putting it in a static block
- does not harm on storm submitter as it just loads `DriverManager` class
- it avoids going through synchronized in prepare calls of all JDBC tasks
in a worker though it may not involve much cost as prepare is invoked only once
for each task's life.
I am fine with either ways. :)
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
