oracle / node-oracledb

Oracle Database driver for Node.js maintained by Oracle Corp.
http://oracle.github.io/node-oracledb/
Other
2.24k stars 1.07k forks source link

Why thin/pool _scanIdleConnection pops only last _freeConnectionList item? #1633

Closed boriborm closed 3 months ago

boriborm commented 6 months ago

Hello, File thin/pool.js has procedure

//---------------------------------------------------------------------------
  // scanIdleConnection()
  //
  // scan connection list and removes idle connections from pool
  //---------------------------------------------------------------------------
  _scanIdleConnection() {
    while ((this._usedConnectionList.size + this._freeConnectionList.length) >
        this._poolMin && this._freeConnectionList.length > 0) {
      const conn = this._freeConnectionList[this._freeConnectionList.length - 1];
      if (Date.now() - conn._lastTimeUsed < this._poolTimeout * 1000) {
        break;
      }

      this.eventEmitter.emit('_removePoolConnection', conn);
      this._freeConnectionList.pop();
    }

    this._schedulerJob = null;
    this._setScheduler();
  }

Why in cycle pops only last elements of array _freeConnectionList? I have some web requests. Each request get connection from the pool. First connection releases and pushes to _freeConnectionList and second connection pushes to _freeConnectionList. _lastTimeUsed of first connection may be earlier then second connection. While second connection did not timeout, while breakes and first connection with timeout will not pop. Other requests (not in same time) get new connection from last element of _freeConnectionList. This is real second connection. _lastTimeUsed of second connection changes.

sudarshan12s commented 6 months ago

Hi @boriborm , We see that the current the scanning of _freeConnectionList skips removing idle connections in some scenarios. We are changing this function to scan from beginning _freeConnectionList[0] , ... and break if we found a connection that got expired. We are working on this and will share more details...

As per the_lastTimeUsed, we set it in release and push into the _freeConnectionList and scan would start removing idle connections from other end. Since c1.close and c2.close are async, the order they get pushed to _freeConnectionList may differ from the order they got called...

I hope this clarifies your query?

sudarshan12s commented 6 months ago

Hi @boriborm , for the idle connection removal in correct order, it needs a small patch as below. We shall include in coming release. Thanks.

index 97c754be..1367f92a 100644
--- a/lib/thin/pool.js
+++ b/lib/thin/pool.js
@@ -363,13 +363,13 @@ class ThinPoolImpl extends PoolImpl {
   _scanIdleConnection() {
     while ((this._usedConnectionList.size + this._freeConnectionList.length) >
         this._poolMin && this._freeConnectionList.length > 0) {
-      const conn = this._freeConnectionList[this._freeConnectionList.length - 1];
+      const conn = this._freeConnectionList[0];
       if (Date.now() - conn._lastTimeUsed < this._poolTimeout * 1000) {
         break;
       }

       this.eventEmitter.emit('_removePoolConnection', conn);
-      this._freeConnectionList.pop();
+      this._freeConnectionList.shift();
     }

     this._schedulerJob = null;
sharadraju commented 3 months ago

@boriborm This has been fixed in node-oracledb 6.4.