Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL Bind/Cursor Issue #1694

Open
slewis30328 opened this issue Sep 23, 2024 · 14 comments
Open

SQL Bind/Cursor Issue #1694

slewis30328 opened this issue Sep 23, 2024 · 14 comments
Labels

Comments

@slewis30328
Copy link

We have encountered an issue related to max open cursors. We have mapped the error back to a specific query. The query is a little different - binds to a collection type. Unfortunately, we are unable to replicate this. The problem has only surfaced 2X in the last month - obviously volume/load related. The internal references to Oracle packages is interesting. Not familiar with that package. We recently upgraded from Oracle 19.12 to 19.24. I'm not 100% sure of this, but I don't think we ran into this issue prior to the upgrade. Any ideas would be much appreciated.

NOTE: :p_catMasterIDArr would never have more than 100 elements

Query:
SELECT CATMASTERID
FROM SMSYS.ITEMFAV
WHERE LOCATIONID = :p_locID
AND CATMASTERID IN (SELECT * FROM TABLE(:p_catMasterIDArr))

Oracle Error:
09/19/2024 12:03:29 ORA-01000: maximum open cursors exceeded
ORA-06512: at "SYS.DBMS_PICKLER", line 144
ORA-06512: at "SYS.DBMS_PICKLER", line 189
ORA-06512: at "SYS.DBMS_PICKLER", line 405
ORA-06512: at "SYS.DBMS_PICKLER", line 144

Application Env:
oracledb version: 6.5.1
Node version: node-v20.11.0-win-x64
Oracle DB version: 19.24.0.0.0
OS: Windows Server 2016

@slewis30328
Copy link
Author

Just wanted to add an update to this issue. After further testing, I can see that the cursor is never released/reused on this query. Every time the query runs, a new row is created in v$open_cursor. All my other queries are parsed and loaded 1X and I do not see any additions to v$open_cursor. I think this issue should be flagged as a bug at this time.

@sudarshan12s
Copy link

sudarshan12s commented Sep 24, 2024

Thanks @slewis30328 for the details, we will try to reproduce it . Can you give more details :

  • Is the query run from pl/sql block? I mean like below or its just a sql query. any minimal program would help.
  • I believe its run in thin mode not in thick mode (oracledb.initOracleClient )

const tableName = 'itemfav';
const typename = 'catmasterid_arr';

const sql = `          
-- Create a table similar to SMSYS.ITEMFAV
CREATE TABLE if not exists ${tableName} (
    catmasterid NUMBER,
    locationid NUMBER
)`;
    await connection.execute(sql);

    const binds = Array.from({ length: 100 }, (_, index) => [index + 1, 100]);

    await connection.executeMany(`INSERT INTO ${tableName} (catmasterid, locationid) VALUES (:1, :2)`, binds);

await connection.execute(`
                -- Create a PL/SQL collection type
                CREATE OR REPLACE TYPE ${typename} IS TABLE OF NUMBER;`);

await connection.execute(`-- PL/SQL anonymous block with collection bind
                    DECLARE
                        p_catMasterIDArr ${typename} := ${typename}();
                        p_locID NUMBER := 100;
                        CURSOR c1 IS
                            SELECT catmasterid
                            FROM ${tableName}
                            WHERE locationid = p_locID
                            AND catmasterid IN (SELECT * FROM TABLE(p_catMasterIDArr));
                        v_catMasterID NUMBER;
                    BEGIN
                        -- Populate the collection with some test data
                        FOR i IN 1..100 LOOP
                            p_catMasterIDArr.EXTEND;
                            p_catMasterIDArr(i) := i;
                        END LOOP;
                    
                        -- Open the cursor and fetch results
                        OPEN c1;
                        LOOP
                            FETCH c1 INTO v_catMasterID;
                            EXIT WHEN c1%NOTFOUND;
                            DBMS_OUTPUT.PUT_LINE('CATMASTERID: ' || v_catMasterID);
                        END LOOP;
                    
                        -- Close the cursor
                        CLOSE c1;
                    END;`);

@slewis30328
Copy link
Author

It's just run as a sql query and we are using thin mode.

Here is some DDL examples if needed:

CREATE OR REPLACE type SMSYS.NumberArray as table of number

CREATE TABLE SMSYS.ITEMFAV
(
ITEMFAVID NUMBER NOT NULL,
LOCATIONID NUMBER NOT NULL,
CATMASTERID NUMBER NOT NULL,
ITEMFAVCATID NUMBER NOT NULL,
CREATEDATE DATE NOT NULL,
CREATEUSER VARCHAR2(50 BYTE) NOT NULL,
UPDATEDATE DATE,
UPDATEUSER VARCHAR2(50 BYTE)
)

NOTE: I didn't add index DDL - let me know if you need that.

@sudarshan12s
Copy link

sudarshan12s commented Sep 25, 2024

I am trying to add a test as below to reproduce the issue seen , can you check and let me know if it represents the issue at high level.


'use strict';

const oracledb = require('oracledb');
const dbConfig = require('./dbconfig.js');

const getOpenCursorCount = async function(systemconn, sid) {
  const sql = `
     select ss.value
     from v$sesstat ss, v$statname sn
     where ss.sid = :sid
       and ss.statistic# = sn.statistic#
       and sn.name = 'opened cursors current'`;
  const result = await systemconn.execute(sql, [sid]);
  return result.rows[0][0];  // open cursor count so far in the session
};

const getSid = async function(conn) {
  const sql = `select sys_context('userenv','sid') from dual`;
  const result = await conn.execute(sql);
  return result.rows[0][0];  // session id
};

async function run() {

  let connection, sysDBAConn;
  const tableName = 'itemfav';
  const typename = 'CATMASTERIDARR'; // keep in capital

  try {
    connection = await oracledb.getConnection(dbConfig);

    const dbaConfig = {
      user: dbConfig.DBA_user,
      password: dbConfig.DBA_password,
      connectionString: dbConfig.connectString,
      privilege: oracledb.SYSDBA
    };
    sysDBAConn = await oracledb.getConnection(dbaConfig);
    const sid = await getSid(connection);

    let openCount = await getOpenCursorCount(sysDBAConn, sid);
    console.log(' open cursor count to start ', openCount);

    let sql = `
-- Create a table similar to SMSYS.ITEMFAV
CREATE TABLE  ${tableName} (
    catmasterid NUMBER,
    locationid NUMBER
)`;
    await connection.execute(sql);

    const binds = [];
    for (let index = 0; index < 100; index++) {
      binds.push([index + 1, 100]);
    }
    await connection.executeMany(`INSERT INTO ${tableName} (catmasterid, locationid) VALUES (:1, :2)`, binds);

    sql = ` -- Create a PL/SQL collection type
                CREATE OR REPLACE TYPE ${typename} IS TABLE OF NUMBER;`;
    await testsUtil.createType(connection, typename, sql);

    const objData = Array.from({ length: 100 }, (_, index) => index + 1);
    const objClass = await connection.getDbObjectClass(typename);
    const testObj = new objClass(objData);

    // try increased load by looping 
    const iterations = 10000;  // (increase as needed)

let result;
    for (let i = 1; i <= iterations; i++) {
      result = await connection.execute(`SELECT catmasterid
    FROM ${tableName}
    WHERE locationid = 100
    AND catmasterid IN (SELECT * FROM TABLE(:p_catMasterIDArr))`, [testObj]);
      //console.log(result);
    }

    openCount = await getOpenCursorCount(sysDBAConn, sid);
    console.log(' open count after  all iterations ', openCount);

result = await sysDBAConn.execute(`SELECT
    SID,
    USER_NAME,
    ADDRESS,
    HASH_VALUE,
    SQL_ID,
    SQL_TEXT,
    CURSOR_TYPE
FROM
    V$OPEN_CURSOR
WHERE
    SID = ${sid}`);
    console.log(`details of open cursors `, result.rows);

  } catch (err) {
    console.log(err);
  } finally {
    await connection.execute(`drop TABLE ${tableName}`);
    await connection.close();
    await sysDBAConn.close();
  }
}

run(); 

@sudarshan12s
Copy link

@slewis30328 , I could not reproduce the issue with 19.24 DB version. Can you let us know if above program reproduces the issue in your environment or if the above test needs modifications to reproduce.

@slewis30328
Copy link
Author

When I get a chance, I will see if I can reproduce issue with your test case above. Not sure if this makes a difference, but we are using connection pooling - example above is not using that. When I run my tests, this is the SQL statement that gets repeated and causes the open cursor count to increase for each execution. I guess this statement is run from your driver layer?

SELECT /*+ NOPARALLEL */
1,
A.NAME,
A.ATTRIBUTE#,
DECODE (AT.TYPECODE,
9, DECODE (A.CHARSETFORM, 2, 'NVARCHAR2', ATO.NAME),
96, DECODE (A.CHARSETFORM, 2, 'NCHAR', ATO.NAME),
112, DECODE (A.CHARSETFORM, 2, 'NCLOB', ATO.NAME),
ATO.NAME),
DECODE (BITAND (AT.PROPERTIES, 64), 64, NULL, ATU.NAME),
NULL,
A.ATTR_TOID,
DECODE (BITAND (T.PROPERTIES, 65536), 65536, 'NO', 'YES'),
SU.NAME,
SO.NAME
FROM SYS.ATTRIBUTE$ A,
SYS.TYPE$ T,
SYS.TYPE$ AT,
SYS."_CURRENT_EDITION_OBJ" ATO,
SYS.USER$ ATU,
SYS."_CURRENT_EDITION_OBJ" SO,
SYS.USER$ SU
WHERE T.TVOID = :B1
AND A.ATTR_TOID = ATO.OID$
AND ATO.OWNER# = ATU.USER#
AND A.TOID = T.TVOID
AND T.PACKAGE_OBJ# IS NULL
AND AT.TVOID = A.ATTR_TOID
AND AT.SUPERTOID = SO.OID$(+)
AND SO.OWNER# = SU.USER#(+)
ORDER BY ATTRIBUTE#

@sudarshan12s
Copy link

sudarshan12s commented Sep 25, 2024

When I get a chance, I will see if I can reproduce issue with your test case above. Not sure if this makes a difference, but we are using connection pooling - example above is not using that. When I run my tests, this is the SQL statement that gets repeated and causes the open cursor count to increase for each execution. I guess this statement is run from your driver layer?

SELECT /*+ NOPARALLEL */ 1, A.NAME, A.ATTRIBUTE#, DECODE (AT.TYPECODE, 9, DECODE (A.CHARSETFORM, 2, 'NVARCHAR2', ATO.NAME), 96, DECODE (A.CHARSETFORM, 2, 'NCHAR', ATO.NAME), 112, DECODE (A.CHARSETFORM, 2, 'NCLOB', ATO.NAME), ATO.NAME), DECODE (BITAND (AT.PROPERTIES, 64), 64, NULL, ATU.NAME), NULL, A.ATTR_TOID, DECODE (BITAND (T.PROPERTIES, 65536), 65536, 'NO', 'YES'), SU.NAME, SO.NAME FROM SYS.ATTRIBUTE$ A, SYS.TYPE$ T, SYS.TYPE$ AT, SYS."_CURRENT_EDITION_OBJ" ATO, SYS.USER$ ATU, SYS."_CURRENT_EDITION_OBJ" SO, SYS.USER$ SU WHERE T.TVOID = :B1 AND A.ATTR_TOID = ATO.OID$ AND ATO.OWNER# = ATU.USER# AND A.TOID = T.TVOID AND T.PACKAGE_OBJ# IS NULL AND AT.TVOID = A.ATTR_TOID AND AT.SUPERTOID = SO.OID$(+) AND SO.OWNER# = SU.USER#(+) ORDER BY ATTRIBUTE#

Thanks. I tried with pool. I modified my test using pool. I am able to see the issue with connection been used to create a dbobject, release to pool in loop , I will share more details.


for (let i = 1; i <= iterations; i++) {
      connection = await pool.getConnection();
      const objClass = await connection.getDbObjectClass(typename);
      const testObj = new objClass(objData);
      const result = await connection.execute(`SELECT catmasterid
    FROM ${tableName}
    WHERE locationid = 100
    AND catmasterid IN (SELECT * FROM TABLE(:p_catMasterIDArr))`, [testObj]);
      await connection.close();
    }

@sudarshan12s
Copy link

I am able to reproduce the issue. The cursor close in this case was missing. This is not a complete fix, but only for above specific case. If its possible to test this patch in the test env, please confirm. We will share the complete fix.

diff --git a/lib/thin/connection.js b/lib/thin/connection.js
index 6d9d4f05d..f06f3da49 100644
--- a/lib/thin/connection.js
+++ b/lib/thin/connection.js
@@ -543,8 +543,10 @@ class ThinConnectionImpl extends ConnectionImpl {
     // check cache; if already present, nothing more to do!
     const info = this._getDbObjectType(result.outBinds.schema,
       result.outBinds.name, result.outBinds.package_name, result.outBinds.oid);
-    if (!info.partial)
+    if (!info.partial) {
+      result.outBinds.attrs_rc.close();
       return info;
+    }

     // process TDS and attributes cursor
     if (info.name.endsWith('%ROWTYPE')) {
@@ -610,6 +612,7 @@ class ThinConnectionImpl extends ConnectionImpl {
       }
       await this._parseTDS(result.outBinds.tds, info);
     }
+    result.outBinds.attrs_rc.close();
     info.partial = false;
     return info;

@slewis30328
Copy link
Author

Can I apply this to 6.6.0 or is this just for 6.5.1? My dev env is 6.6.0 and we have 6.5.1 in TEST and PROD.

@slewis30328
Copy link
Author

I tested with 6.5.1 and that fix above worked. Do not see the cursor count increase.

@sudarshan12s sudarshan12s added bug and removed question labels Sep 26, 2024
@slewis30328
Copy link
Author

I know you said this a temp fix, but could I apply this in our PROD env until the patch comes out? When the count is exceeded, this causes major issues at application level. If you don't think that's a good idea, I could always go back to a standard IN clause. Not crazy about the use of that based on potential param size - much cleaner approach binding to the collection.

@sudarshan12s
Copy link

I have done more testing. I am sharing the patch for 6.5.1 . The issue was happening when the getDbObjectClass was done on a given type multiple times between connection acquire and release to pool.
Yes it is safe to be applied in production. This fix will be available in the next release. Thanks.

diff --git a/lib/thin/connection.js b/lib/thin/connection.js
index 7a8a90548..db9ca7904 100644
--- a/lib/thin/connection.js
+++ b/lib/thin/connection.js
@@ -513,31 +513,37 @@ class ThinConnectionImpl extends ConnectionImpl {
     // check cache; if already present, nothing more to do!
     const info = this._getDbObjectType(result.outBinds.schema,
       result.outBinds.name, result.outBinds.package_name, result.outBinds.oid);
-    if (!info.partial)
+    if (!info.partial) {
+      result.outBinds.attrs_rc.close();
       return info;
+    }

+    try {
     // process TDS and attributes cursor
-    info.version = result.outBinds.version;
-    const attrRows = await result.outBinds.attrs_rc.getRows(1000, {});
-    if (attrRows.length > 0) {
+      info.version = result.outBinds.version;
+      const attrRows = await result.outBinds.attrs_rc.getRows(1000, {});
+      if (attrRows.length > 0) {
       // Its an object not a collection.
-      info.attributes = [];
-      for (const row of attrRows) {
-        const attr = { name: row[1] };
-        if (row[4]) {
-          attr.type = types.DB_TYPE_OBJECT;
-          attr.typeClass = this._getDbObjectType(row[4], row[3], row[5], row[6]);
-          if (attr.typeClass.partial) {
-            this._partialDbObjectTypes.push(attr.typeClass);
+        info.attributes = [];
+        for (const row of attrRows) {
+          const attr = { name: row[1] };
+          if (row[4]) {
+            attr.type = types.DB_TYPE_OBJECT;
+            attr.typeClass = this._getDbObjectType(row[4], row[3], row[5], row[6]);
+            if (attr.typeClass.partial) {
+              this._partialDbObjectTypes.push(attr.typeClass);
+            }
+          } else {
+            attr.type = types.getTypeByColumnTypeName(row[3]);
           }
-        } else {
-          attr.type = types.getTypeByColumnTypeName(row[3]);
+          info.attributes.push(attr);
         }
-        info.attributes.push(attr);
       }
-    }

-    await this._parseTDS(result.outBinds.tds, info);
+      await this._parseTDS(result.outBinds.tds, info);
+    } finally {
+      result.outBinds.attrs_rc.close();
+    }
     info.partial = false;
     return info;

Note: I have added finally block to close in all cases. Without tabs (git diff -w) , the diff is minor

@slewis30328
Copy link
Author

Is it possible to post the updated connection.js file from your patch? Thank you.

@sudarshan12s
Copy link

sudarshan12s commented Sep 29, 2024

Is it possible to post the updated connection.js file from your patch? Thank you.

connection.txt

Please rename to connection.js and place it in lib/thin/ folder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants