Connection pooling

[Aaron Bartell]

You can learn more of the need for connection pools by looking at this thread which includes the eventual solution. I also wrote an article about the topic in MCPressOnline.

This Issue is to discuss:

• Whether there is agreement to add connection pool capabilities to this repo (I believe it should be included).
• What features should be in a connection pool.
• Whether the code in this article is "good enough" for an initial implementation.
• Implications for the Node.js iToolkit if any.

UPDATE 2017-08-30* There are discussions in this issue that will effect this issue.

[Former user]

OK, my opinion, brutal short of it ...

My proposal

First, please see issue db2sock impact node db2 driver for my real opinion about 'repeal and replace' db2 driver and toolkit (forgive political parlance).

About right now

However, right now (today), current design of both db2 and toolkit driver with respect to 'connection' is flawed. To wit, they lead users to make a critical error about connection pooling required for 'async' interfaces, due to DB2 CLI not thread safe ( talk to IBM i Java JDBC expert next Common, if you doubt DB2/CLI 'not thread safe' ).

Wrong way

In fact, the IBM i web page demonstration of 'async' is incorrect, and, subject to random fails (if followed as pattern). Specifically, db2 'async' sample on IBM i OPS web site sharing one connection (dbconn) with multiple statements (sqlA, sqlB) is wrong! Technically, exec below runs 'async', therefore timing dependent possible 2 threads breaks db2 golden rule “no share conn/stmt across threads”. You are at mercy of probability theory. Spinning roulette wheel. Right way, use two connections (dbconn1/sqlA, dbconn2/sqlB). The bears demonstration litmis/nodejs shows better sample pattern with connection pooling.

var db = require('./db2a');
var dbconn = new db.dbconn();
var sql = "SELECT * FROM QIWS.QCUSTCDT";
dbconn.conn("*LOCAL");
var sqlA = new db.dbstmt(dbconn); // no -- bad idea two stmts on one conn
var sqlB = new db.dbstmt(dbconn); // no -- bad idea two stmts on one conn
console.log("Execute A.");
sqlA.exec(sql, function() {
  console.log("Execute A Done.");
  delete sqlA;
});
console.log("Execute B.");
sqlB.exec(sql, function() {
  console.log("Execute B Done.");
  delete sqlB;
});

Why does test above work??? It is random. The test can operate quickly enough that it may even run in same 'worker' thread for both operations. Aka, we are not fetching the result, so we are running really fast. Also, console.log() messages are introducing a 'false sync' to output coherent messages to the terminal (screen). In the end, do not follow the pattern, you are just getting lucky.

Right way

To demonstrate simple change needed (below), db2a interface requires nested connection-pool through fetch render as 'pyramid of doom' (popular term node nested functions).

// someday future via db2sock
// https://bitbucket.org/litmis/db2sock
global._db2 = require('/QOpenSys/QIBM/ProdData/OPS/Node6/os400/db2i/lib/db2a_future');
function task(who,waitInterval) {
  _db2.pool(function(conn) {
    conn.stmt(function(stmt) {
      sql = "select LSTNAM, STATE from QIWS.QCUSTCDT where LSTNAM='"+who+"'";
      stmt.prepare(sql, function() {
        stmt.execute(function(){
          stmt.fetchAll(function(result){ 
            setTimeout(function(){
              global._cb(result);
              delete stmt;
            }, waitInterval, result);
          });
        });
      });
    });
  });
  return 0;
};
var task1 = task('Jones',5000); // task 1 (async)
var task2 = task('Vine',3000); // task 2  (async)

[Aaron Bartell]

First, please see issue db2sock impact node db2 driver for my real opinion about 'repeal and replace' db2 driver and toolkit (forgive political parlance).

I've place an addendum in the description of this issue per the db2sock stuff.

[Former user]

The node toolkit suffers from this same db2a 'async' design flaw. That is, current design leads to non-optimal, and/or 'un-safe' use of db2 connections.

Again, 'repeal and replace' using db2sock is probably the best answer (opinion).

However, you can use connection pools in toolkit as well (of a sort). Although, with connection buried inside toolkit.js class, this may fail under stress.

// connection pool golden rule 
// -- never share connection/statement across threads.
// -- async db2 uses threads (always)
global._xt = require('/QOpenSys/QIBM/ProdData/OPS/Node6/os400/xstoolkit/lib/itoolkit.js');
global._cb = function(result){ console.log(result); }
var conn1 = new global._xt.iConn("*LOCAL");
var conn2 = new global._xt.iConn("*LOCAL");
function task(conn,who,waitInterval) {
  conn.add(global._xt.iSh("sleep "+waitInterval+"; date; echo '"+who+"';"));
  conn.run(function (result) {
    var str_json = global._xt.xmlToJson(result);
    global._cb(str_json);
  });
  return 0;
};
var task1 = task(conn1,'Jones',5); // task 1 (async)
var task2 = task(conn2,'Vine',3); // task 2  (async)

[Aaron Bartell]

The node toolkit suffers from this same db2a 'async' design flaw.

Agreed. This is becoming more evident as shops move things into production. I've had multiple scenarios where it was simpler to just run iToolkit RPG calls as sync to stave off unpredictability (I felt dirty forcing things to run sync. Node.js programmers unite to lynch Aaron.)

[Jesse G]

With the new idb-pconnector project, the trap of sharing resources across threads is less likely to occur, for two reasons:

1. Users are expected to use the "await" keyword, which would protect them
2. The idb-pconnector has a Statement constructor that we can advise people to use (or will shortly when changes are committed), which will implicitly get a connection. While this doesn't actually solve any problems, it can help steer users away from concurrent connection-sharing if that becomes the norm in documented examples

var dba = require("idb-pconnector");
async function runInsertAndSelect() {
    try {
        var dbStmt =  new dba.Statement();
        await dbStmt.prepare("INSERT INTO MYSCHEMA.TABLE1 VALUES (?,?)");
        await dbStmt.bind([ [2018,dba.PARM_TYPE_INPUT,2], ['Dog' ,dba.PARM_TYPE_INPUT, 1] ]);
        await dbStmt.execute();
        var res = await dbStmt.exec("SELECT * FROM MYSCHEMA.TABLE1");
        console.log("Select results: "+JSON.stringify(res));
    } catch(dbError) {
        console.log("Error is " + dbError);
        console.log(error.stack);
    } 
}

runInsertAndSelect();

I do, however, believe that connection pooling would be a useful enhancement. I would propose adding this to the idb-pconnector project, which would then let it be used by the Statement constructor when it implicitly gets a connection.

Thoughts @aaronbartell, @rangercairns, and @abmusse?

[Musse]

The Connection Pool is a great idea!

I'm currently working on an implementation for the idb-pconnector.

[Aaron Bartell]

I am late to the game. Still recovering from tradeshow season.

My vote: Yes, let's get connection pooling added.

[Brian Jerome]

@abmusse Do you think it's worth refactoring the idbp-connector pooling code into this project? I'm not sure what status either project is in compared to the nodejs-itoolkit project.

[Musse]

@bjerome probably not worth while because the idb-pconnector implementation being available.

In any event, if anyone would like to take this up we are always open to PRs.