oracle-r2dbc icon indicating copy to clipboard operation
oracle-r2dbc copied to clipboard

OracleReactiveJdbcAdaptor.publishConnection should timeout if Oracle JDBC connection publisher does not resolve

Open volcanic-tephra opened this issue 1 year ago • 1 comments

When creating a connection using the r2dbc ConnectionFactoryOptions the Oracle adaptor will use ForkJoinPool.commonPool() as the executor.

Note: I raised this then noticed PR https://github.com/oracle/oracle-r2dbc/pull/131. However the suggestion below would be complimentary and make the solution more robust (both a default fallback and an eventual exception if it still fails).

If the executor has no free threads then the request to create a connection will hang indefinitely. The publishConnection Publisher should timeout and throw an exception as there is currently zero information that this is the cause of the failure.

The timeout default could be reasonably long, there just needs to be some feedback to troubleshoot.


  public Publisher<? extends Connection> publishConnection(
    DataSource dataSource, Executor executor) {

    long timeout = 10000; // Get from configuration
    OracleDataSource oracleDataSource = unwrapOracleDataSource(dataSource);
    return Mono.from(adaptFlowPublisher(() ->
        oracleDataSource
          .createConnectionBuilder()
          .executorOracle(executor)
          .buildConnectionPublisherOracle()))
      .timeout(Duration.ofMillis(timeout)) // PREVENT THIS FROM HANGING FOREVER!!!
      .onErrorMap(R2dbcException.class, error ->
        error.getErrorCode() == 18714 // ORA-18714 : Login timeout expired
          ? new R2dbcTimeoutException(error.getMessage(),
              error.getSqlState(), error.getErrorCode(), error.getCause())
          : error);
  }

The work around is to create a custom Executor and supply this to the Connection factory.

var executor = Executors.newFixedThreadPool(threads);

var factory = ConnectionFactories.get(
        ConnectionFactoryOptions.builder()
          .option(OracleR2dbcOptions.EXECUTOR, executor)
          .option(ConnectionFactoryOptions.DRIVER, "pool")
          .option(ConnectionFactoryOptions.PROTOCOL, "oracle")
          .option(ConnectionFactoryOptions.HOST, properties.getHost())
          .option(ConnectionFactoryOptions.PORT, properties.getPort())
          .option(ConnectionFactoryOptions.DATABASE, properties.getDatabase())
          .option(ConnectionFactoryOptions.USER, properties.getUser())          
          .option(ConnectionFactoryOptions.PASSWORD, properties.getPassword())
          .build());

volcanic-tephra avatar Oct 20 '23 03:10 volcanic-tephra

Hi @volcanic-tephra. Thanks for reporting this and looking into a timeout based fix.

Do you know which version of the JDK you're using? If its the same "openjdk version "17" 2021-09-14 " seen in #129, that could explain the zero-threaded ForkJoinPool.

If we are seeing the same issue, then the best fix is to update the JDK. But that might not be feasible in all cases. The next best fix is either configuring a non-default Executor, like in your example, or using a system property to configure ForkJoinPool.commonPool() with a single thread: "-Djava.util.concurrent.ForkJoinPool.common.parallelism=1".

Configuring a timeout is a good practice to follow as well.

Michael-A-McMahon avatar Oct 20 '23 18:10 Michael-A-McMahon