Make sharded pool connections respect queueTimeout when poolMaxPerShard has been reached

Author: Christopher Jones

CHANGELOG.md

@@ -22,6 +22,9 @@
 - Fixed various execution failures with Node.js 13.2 due to Node.js NULL pointer behavior change ([ODPI-C
   change](https://github.com/oracle/odpi/commit/7693865bb6a98568546aa319cc0fdb9e208cf9d4)).
 
+- Fixed connection pooling so sharded `pool.getConnection()` requests respect
+  `queueTimeout` when `poolMaxPerShard` has been reached.
+
 - Added a directory the binary module search to help Webpack use, though a copy
   plugin is still required, see
   [here](https://github.com/oracle/node-oracledb/issues/1156#issuecomment-571554125).

lib/oracledb.js

@@ -174,9 +174,22 @@ function createPool(poolAttrs, createPoolCb) {
   // be called by the database when a connection acquired from the pool doesn't
   // have the requested tag
   sessionCallback = poolAttrs.sessionCallback;
+
+  // create an adjusted set of pool attributes to pass to the C layer; the
+  // session callback must be removed if it is a JavaScript function and the
+  // queue timeout is used to specify the maximum amount of time that the C
+  // layer will wait for a connection to be returned; ordinarily since the
+  // JavaScript layer never calls the C layer to get a connection unless one is
+  // known to be available, this should not be needed, but in some cases (such
+  // as when the maximum for a particular shard is specified) this may not be
+  // known, so this prevents an unnecessarily long wait from taking place
+  const adjustedPoolAttrs = Object.defineProperties({},
+    Object.getOwnPropertyDescriptors(poolAttrs));
   if (typeof poolAttrs.sessionCallback === 'function') {
-    poolAttrs = Object.assign({}, poolAttrs);
-    delete poolAttrs.sessionCallback;
+    delete adjustedPoolAttrs.sessionCallback;
+  }
+  if (adjustedPoolAttrs.queueTimeout === undefined) {
+    adjustedPoolAttrs.queueTimeout = self.queueTimeout;
   }
 
   // Need to prevent another call in the same stack from succeeding, otherwise
@@ -186,7 +199,7 @@ function createPool(poolAttrs, createPoolCb) {
     tempUsedPoolAliases[poolAlias] = true;
   }
 
-  self._createPool(poolAttrs, function(err, poolInst) {
+  self._createPool(adjustedPoolAttrs, function(err, poolInst) {
     if (err) {
       // We need to free this up since the creation of the pool failed.
       if (poolAlias) {
@@ -208,7 +221,6 @@ function createPool(poolAttrs, createPoolCb) {
       delete tempUsedPoolAliases[poolAlias];
     }
 
-    poolAttrs.sessionCallback = sessionCallback;
     poolInst._setup(poolAttrs, poolAlias, self);
 
     poolInst.on('_after_close', function() {

src/njsModule.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+// Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
 
 //-----------------------------------------------------------------------------
 //
@@ -426,6 +426,7 @@ struct njsBaton {
     uint32_t poolMaxPerShard;
     uint32_t poolIncrement;
     uint32_t poolTimeout;
+    uint32_t poolWaitTimeout;
     int32_t poolPingInterval;
     uint32_t stmtCacheSize;
     uint32_t maxRows;

src/njsOracleDb.c

@@ -1,4 +1,4 @@
-// Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
+// Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
 
 //-----------------------------------------------------------------------------
 //
@@ -367,7 +367,9 @@ static bool njsOracleDb_createPoolAsync(njsBaton *baton)
     params.maxSessions = baton->poolMax;
     params.maxSessionsPerShard = baton->poolMaxPerShard;
     params.sessionIncrement = baton->poolIncrement;
-    params.getMode = DPI_MODE_POOL_GET_WAIT;
+    params.getMode = (baton->poolMaxPerShard > 0) ?
+            DPI_MODE_POOL_GET_TIMEDWAIT : DPI_MODE_POOL_GET_WAIT;
+    params.waitTimeout = baton->poolWaitTimeout;
     params.externalAuth = baton->externalAuth;
     params.homogeneous = baton->homogeneous;
     params.plsqlFixupCallback = baton->plsqlFixupCallback;
@@ -495,6 +497,9 @@ static bool njsOracleDb_createPoolProcessArgs(njsBaton *baton, napi_env env,
     if (!njsBaton_getBoolFromArg(baton, env, args, 0, "events",
             &baton->events, NULL))
         return false;
+    if (!njsBaton_getUnsignedIntFromArg(baton, env, args, 0, "queueTimeout",
+            &baton->poolWaitTimeout, NULL))
+        return false;
 
     return true;
 }