Dev Branch available with Promises - Node-oracledb 1.9.x Plans

[cjbj]

It's been a while since I did a road map for a release.

Note the call-to-action to help test.

Our current thoughts for the 1.9.x series are:

• Push out a developer branch ("1.9.0") to GitHub with @dmcghan's Promise and related JavaScript work such as close aliases, allowing REF CURSORS to be streamed, a prototype way to allow query streams to be closed without fetching all results, and few bits and pieces. There would NOT be an npm release.
• get feedback on the API in this dev branch and stabilize it.
• Test suite, doc etc PR's from anyone who has/will sign the OCA will help speed up the release process. Feedback on 3rd party Promise libraries will be useful since we will only be looking at native Promises (we have to draw the line somewhere!). Since the Promise API affects all methods, testing thoroughly is currently expected to take most time.
• When the JS layer is OK, merge the JS work and any C++ layer fixes we have completed separately. I'm hopeful we will have fixed the RS mem leak.
• Push a 1.9.1 Production release to NPM. This may possibly be 2 - 4 weeks after the JS dev branch is pushed, depending on bike-shedding or issues uncovered with the Promise layer. No guarantees, of course.

How does that sound? Obviously if something is taking time and we have other fixes ready we can adjust the order.

[sagiegurari]

What do you mean when you write "when js layer is OK"?

[cjbj]

@sagiegurari I mean when you (and other users) are OK with the design & implementation, and when there is good Promise test coverage and the docs have been fully updated.

[sagiegurari]

Thanks

[sagiegurari]

few recommendations for a general roadmap:

• tests - the tests do require('oracledb') instead of require('..') so if you want to run them, you must do git clone to a folder named oracledb and put that in a folder named node_modules. it is strange and should be fixed. i want to do git clone to any folder and run npm test and it should work.
• docs - hard to maintain them. all the numbering is very not markup/github friendly. can be simplified a lot.
• missing doc to help PR developers see what steps/files they need to modify apart of the code that they push. the repo got some strange files that hold lists of things that i'm not even sure if used or not like in the tests folder.
• the tests are not stable. they don't always work. i can't explain it because i didn't investigate but for PR developers it is problematic. i wasn't sure if i messed something up or not.
• missing continues integration. for PR developer doing a pull request, it is nice to see that the official build passes. I know travis might not work for this type of library, but maybe something else will. it is very important.
• this is not development task, but in general, PRs should be merged and not manually rewritten in the internal repo. it is not very rewarding to see someone take the work you did and just manually push it with changes on the way. usually people have pride in the work they do and if there is an issue it should be fixed as part of the PR and merged only once everyone is happy with it.
  not to mention it is limiting visibility which for open source project is strange.

[cjbj]

I've pushed the dev-1.9 branch. Some release notes are here.

[cjbj]

@sagiegurari:

• Tests/examples: I was thinking about reducing the number of tests
  installed via npm, and including some examples. Currently all tests
  are installed but no examples. I'd like to head towards having a few
  simple sanity tests and a few examples installed. The rest of the
  tests & examples would be runnable/installable only from GitHub, so
  changing the require() path would make sense. (Anyone got
  experience with .npmignore files and negation?!!)
• Tests are stable in the various environments we run them. If you have
  diffs, let us know. We can't fix what we can't see.
• Docs are growing and also getting harder to navigate as a reader.
  There have been some discussions about using a different format. Time
  is the limiting factor.
• You and I have discussed the two-repo rationale before. I don't have
  more to add about the restrictions on us, but point taken.
• CI is a good idea but not on the short term plan.

Thanks for taking time to let us know where things stand.

[sagiegurari]

thanks.
.npmignore files are just like .gitignore files. you just select the paths to prevent npm from packaging.
https://docs.npmjs.com/misc/developers

i would also suggest regarding tests, is to check coverage.
the issue is that a big piece of this library is C so i'm not sure how to do that.
for JS it is very simple using istanbul.

[cjbj]

@sagiegurari the npmignore format is supposed to be the same as gitignore "any matching file excluded by a previous pattern will become included again", but my quick tests to try and bundle only a few of the tests either caused all, or no, tests to be bundled.

Regarding coverage, @octokyle is running some code coverage tools on C & JS. Coverage is good but, like all testing, could be improved. I forget the current numbers.

[sagiegurari]

maybe something wrong with the npmignore, but regardless, for readability, I suggest you might want to split the tests to 2 folders.
1 folder for spec and 1 for more deep spec and just exclude the deep folder.

[cjbj]

@sagiegurari simple but effective!

[cjbj]

Naming dropping a few people who have used or mentioned promises in this project @jrop @isian8814 @mrapczynski @wmertens @ecowden @bberryhill @alexsantos @jeffm13

If you have feedback on the Promise support in 1.9.0 Development branch, let us know soon.

[mrapczynski]

@cjbj Brilliant!

[toberoo]

The stream feature looks extremely useful, nice! I have a question with the promise implementation though. How does it work with thread, (meant connection) pools? It's not clear to me looking at the code, can you provide an example ?

[dmcghan]

@toberoo Thanks for the feedback! Do you mean connection pools? There's an example on using Promises here:
https://github.com/oracle/node-oracledb/blob/dev-1.9/examples/promises.js

The only difference with connection pools would be that you get the connection from the pool rather than the base class. Does that help?

[toberoo]

Could I do something like:

oracleDb.createPool({
//Options
})
.then(function(pool) {

})
.catch(function(err) {

});

[dmcghan]

@toberoo Absolutely. Code like that would generally be in your main/app.js. Then you could get and use connections in a manner similar to the example I showed you before.

[cjbj]

It's been a week since the dev branch with Promise support was pushed. We'd love to hear anyone's experience with it. See the release notes. @sagiegurari @jrop @isian8814 @mrapczynski @wmertens @ecowden @bberryhill @alexsantos @jeffm13

[sagiegurari]

from quick review it looks good to me.
I didn't run any tests against that branch however, I'll try to do that.

[ghost]

To me it seems that dev-1.9 hangs if the database connection is lost when the poolMin option is not 0.

Consider the following:

const oracledb = require('oracledb');

function exec(pool) {
    console.log("Executing query.");

    return pool.getConnection()
        .then(conn => conn.execute("BEGIN NULL; END;"))
        .then(result => {
            console.log("Query done.");
        })
        .catch(err => {
            console.log(err);
        });
}

oracledb.createPool({
        connectString: config.connectString,
        password: config.password,
        poolMin: 1, // <- Will not work if != 0.
        poolTimeout: 1,
        user: config.user
    })
    .then(pool => {
        console.log("Unplug your database connection now.");
        setTimeout(() => exec(pool), 10000);
    })
    .catch(err => {
        console.log(err);
    });

It will output the text Executing query. but it will never actually end the Promise. I tried playing with the timeout option, but it had no effect. It probably attempts to open more connections, but as it fails connect the database, it never throws out?

[dmcghan]

@Laphro When I run your example it finishes fine - after 10 seconds I see "Query done" in the console.

If I wait till I see the "Unplug message..." and then shutdown my database, I see "Executing query", then after about 50 seconds or so I get:
[Error: ORA-03113: end-of-file on communication channel
Process ID: 15517
Session ID: 38 Serial number: 140733193421474
]

I'm unable to reproduce the issue.

[cjbj]

@Laphro what OS(s) are you using? What versions? What versions of Oracle DB and client? What are your Oracle Net options and OS TCP timeouts etc?

[cjbj]

@Laphro as Dan mentioned, shutting down the DB or killing the session does return an error so the Promise implementation seems OK. I tried unplugging the cable to my DB - done virtually to a VirtualBox VM. Pausing the VM had the same effect. In this case your code does hang. It hangs either at the execute or the initial, internal 'attach' to the DB, depending on the value of poolMin and when exactly the VM was paused. The same behavior occurs without Promises too. If you don't want to wait for whatever OS level timeout will occur, you could create a sqlnet.ora file on the Node host setting SQLNET.RECV_TIMEOUT (see http://docs.oracle.com/database/121/NETRF/sqlnet.htm#NETRF227). In the shell that starts Node set the environment variable TNS_ADMIN to the directory containing sqlnet.ora. The app will then give you Error: ORA-12609: TNS: Receive timeout occurred.

[ghost]

@cjbj Sorry for not getting back earlier.

I also run both of my OS's on VirtualBox. Node is running on Ubuntu 16.04 VM with the 11.2 client. The database 11.2.0.4.0 is on Windows Server 2012. The hang happens if I turn off the database VM or disconnect the client VM's network connection.

I tried it and the SQLNET.RECV_TIMEOUT parameter worked as you explained. I now got the timeout that I wanted. So the Promise implementation works just fine. Thank you.

[cjbj]

@Laphro thanks for the update. Our Net team reminded me that SQLNET.RECV_TIMEOUT could timeout if you have slow SQL statements too. This may or may not be desirable.

[sagiegurari]

Just ran my tests (found in https://github.com/sagiegurari/simple-oracledb/blob/master/test/spec/integration-spec.js) with dev-1.9 branch and it all passed ok.
So I'm good with this version.

Regarding CI, I read a bit travis-ci and docker.
You can use docker in travis, so you can use some oracle image that has a DB preinstalled and use that to npm install + npm test this package.
I think it should be very helpful.

[cjbj]

@sagiegurari thank you. We'll wrap up 1.9.1 soon.

[jrop]

After a brief test, everything seems to be working with our libs! Thanks for this guys!

[cjbj]

@jrop thank you.

[cjbj]

To keep everyone updated: I'm impatient to release 1.9.1 but cooler heads have prevailed. We are going to take a few more days to soak-test fixes that should stop the garbage collector kicking in too early.

[jeffm13]

Our initial tests show that things are working as expected. We'll try to run through some load testing next week.

[cjbj]

Thanks @jeffm13. You might want to wait for 1.9.1 before doing serious testing; it should have the fix for #307

[cjbj]

Node-oracledb 1.9.1 has been released. Yay! Thanks for all the feedback. Check out the release notes at https://blogs.oracle.com/opal/entry/node_oracledb_1_9_1