feat: Improved connection pool for HANAService (#349)

db-service/lib/common/DatabaseService.js

@@ -63,6 +63,7 @@ class DatabaseService extends cds.Service {
 
     // Acquire a pooled connection
     this.dbc = await this.acquire()
+    this.dbc.destroy = this.destroy.bind(this)
 
     // Begin a session...
     try {
@@ -110,8 +111,20 @@ class DatabaseService extends cds.Service {
    */
   async release() {
     if (!this.dbc) return
-    await this.pool.release(this.dbc)
+    const dbc = this.dbc
     this.dbc = undefined
+    await this.pool.release(dbc)
+  }
+
+  /**
+   * Destroys own connection, i.e. tix.dbc, from this.pool
+   * This is for subclasses to intercept, if required.
+   */
+  async destroy() {
+    if (!this.dbc) return
+    const dbc = this.dbc
+    this.dbc = undefined
+    await this.pool.destroy(dbc)
   }
 
   // REVISIT: should happen automatically after a configurable time

hana/lib/HANAService.js

@@ -29,14 +29,35 @@ class HANAService extends SQLService {
 
   // REVISIT: Add multi tenant factory when clarified
   get factory() {
+    const driver = drivers[this.options.driver || this.options.credentials.driver]?.driver || drivers.default.driver
+    const isMultitenant = 'multiTenant' in this.options ? this.options.multiTenant : cds.env.requires.multitenancy
+    const service = this
     return {
-      options: { max: 1, ...this.options.pool },
-      create: async (/*tenant*/) => {
-        const driver = drivers[this.options.driver || this.options.credentials.driver]?.driver || drivers.default.driver
-        const dbc = new driver(this.options.credentials)
-        await dbc.connect()
-        HANAVERSION = dbc.server.major
-        return dbc
+      options: {
+        min: 0,
+        max: 10,
+        acquireTimeoutMillis: cds.env.profiles.includes('production') ? 1000 : 10000,
+        idleTimeoutMillis: 60000,
+        evictionRunIntervalMillis: 100000,
+        numTestsPerEvictionRun: Math.ceil((this.options.pool?.max || 10) - (this.options.pool?.min || 0) / 3),
+        ...(this.options.pool || {}),
+        testOnBorrow: true,
+        fifo: false
+      },
+      create: async function (tenant) {
+        try {
+          const creds = isMultitenant
+            ? await require('@sap/cds-mtxs/lib').xt.serviceManager.get(tenant, { disableCache: false })
+            : service.options.credentials
+          const dbc = new driver(creds)
+          await dbc.connect()
+          HANAVERSION = dbc.server.major
+          return dbc
+        } catch (err) {
+          if (!isMultitenant || err.code !== 10) throw err
+          await require('@sap/cds-mtxs/lib').xt.serviceManager.get(tenant, { disableCache: true })
+          return this.create(tenant)
+        }
       },
       error: (err /*, tenant*/) => {
         // Check whether the connection error was an authentication error
@@ -52,7 +73,7 @@ class HANAService extends SQLService {
         }
       },
       destroy: dbc => dbc.disconnect(),
-      validate: (/*dbc*/) => true,
+      validate: (dbc) => dbc.validate(),
     }
   }
 
@@ -63,14 +84,18 @@ class HANAService extends SQLService {
     return `hana@${host}:${port}${driver ? `(${driver})` : ''}`
   }
 
+  ensureDBC() {
+    return this.dbc || cds.error`Database is disconnected`
+  }
+
   async set(variables) {
     // REVISIT: required to be compatible with generated views
     if (variables['$valid.from']) variables['VALID-FROM'] = variables['$valid.from']
     if (variables['$valid.to']) variables['VALID-TO'] = variables['$valid.to']
     if (variables['$user.id']) variables['APPLICATIONUSER'] = variables['$user.id']
     if (variables['$user.locale']) variables['LOCALE'] = variables['$user.locale']
 
-    this.dbc.set(variables)
+    this.ensureDBC().set(variables)
   }
 
   async onSELECT({ query, data }) {
@@ -200,11 +225,11 @@ class HANAService extends SQLService {
 
   // prepare and exec are both implemented inside the drivers
   prepare(sql) {
-    return this.dbc.prepare(sql)
+    return this.ensureDBC().prepare(sql)
   }
 
   exec(sql) {
-    return this.dbc.exec(sql)
+    return this.ensureDBC().exec(sql)
   }
 
   /**
@@ -917,12 +942,12 @@ class HANAService extends SQLService {
 
   onCOMMIT() {
     DEBUG?.('COMMIT')
-    return this.dbc.commit()
+    return this.dbc?.commit()
   }
 
   onROLLBACK() {
     DEBUG?.('ROLLBACK')
-    return this.dbc.rollback()
+    return this.dbc?.rollback()
   }
 
   // Creates a new database using HDI container groups

hana/lib/drivers/base.js

@@ -92,7 +92,7 @@ class HANADriver {
 
   set(variables) {
     variables
-    throw new Error('Implementation missing')
+    throw new Error('Implementation missing "set"')
   }
 
   /**
@@ -139,6 +139,14 @@ class HANADriver {
       return prom(this._native, 'disconnect')()
     }
   }
+
+  /**
+   * Validates that the connection is connected
+   * @returns {Promise<Boolean>}
+   */
+  async validate() {
+    throw new Error('Implementation missing "validate"')
+  }
 }
 
 /**

hana/lib/drivers/hana-client.js

@@ -6,14 +6,41 @@ const { driver, prom, handleLevel } = require('./base')
 
 const streamUnsafe = false
 
+const credentialMappings = [
+  { old: 'schema', new: 'currentSchema' },
+  { old: 'certificate', new: 'ca' },
+  { old: 'encrypt', new: 'useTLS' },
+  { old: 'hostname_in_certificate', new: 'sslHostNameInCertificate' },
+  { old: 'validate_certificate', new: 'sslValidateCertificate' },
+]
+
 class HANAClientDriver extends driver {
   /**
    * Instantiates the HANAClientDriver class
    * @param {import('./base').Credentials} creds The credentials for the HANAClientDriver instance
    */
   constructor(creds) {
-    // REVISIT: make sure to map all credential properties for hana-client
-    creds.currentSchema = creds.schema
+    // Enable native @sap/hana-client connection pooling
+    creds = Object.assign({}, creds, {
+      // REVISIT: add pooling related credentials when switching to native pools
+      // Enables the @sap/hana-client native connection pool implementation
+      // pooling: true,
+      // poolingCheck: true,
+      // maxPoolSize: 100, // TODO: align to options.pool configurations
+
+      // If communicationTimeout is not set queries will hang for over 10 minutes
+      communicationTimeout: 60000,
+      // connectTimeout: 1000,
+      // compress: true, // TODO: test whether having compression enabled makes things faster
+      // statement caches come with a side effect when the database structure changes which does not apply to CAP
+      // statementCacheSize: 100, // TODO: test whether statementCaches make things faster
+    })
+
+    // Retain node-hdb credential mappings to @sap/hana-client credential mapping
+    for (const m of credentialMappings) {
+      if (m.old in creds && !(m.new in creds)) creds[m.new] = creds[m.old]
+    }
+
     super(creds)
     this._native = hdb.createConnection(creds)
     this._native.setAutoCommit(false)
@@ -55,6 +82,10 @@ class HANAClientDriver extends driver {
     return ret
   }
 
+  async validate() {
+    return this._native.state() === 'connected'
+  }
+
   _extractStreams(values) {
     // Removes all streams from the values and replaces them with a placeholder
     if (!Array.isArray(values)) return { values: [], streams: [] }
@@ -88,6 +119,8 @@ class HANAClientDriver extends driver {
   }
 }
 
+HANAClientDriver.pool = true
+
 async function* rsIterator(rs, one) {
   const next = prom(rs, 'next') // () => rs.next()
   const getValue = prom(rs, 'getValue') // nr => rs.getValue(nr)

hana/lib/drivers/hdb.js

@@ -22,6 +22,7 @@ class HDBDriver extends driver {
     super(creds)
     this._native = hdb.createClient(creds)
     this._native.setAutoCommit(false)
+    this._native.on('close', () => this.destroy?.())
 
     this.connected = false
   }
@@ -31,6 +32,10 @@ class HDBDriver extends driver {
     Object.keys(variables).forEach(k => clientInfo.setProperty(k, variables[k]))
   }
 
+  async validate() {
+    return this._native.readyState === 'connected'
+  }
+
   /**
    * Connects the driver using the provided credentials
    * @returns {Promise<any>}