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

Add connection pooling and resource cleanup #246

Open
theothermattm opened this issue Jan 21, 2015 · 15 comments
Open

Add connection pooling and resource cleanup #246

theothermattm opened this issue Jan 21, 2015 · 15 comments

Comments

@theothermattm
Copy link

It would be great to have connection pooling built into the node-oracle module along with proper cleanup of resources.

What I mean is, something like this (very rough):

oracle.connectWithPool(connectData, poolOpts);

oracle.acquireConnection().execute("SELECT systimestamp FROM dual", [], function(err, results) {
        if (err) { console.log("Error executing query:", err); return; }

        console.log(results);
    });

Additionally, if promises are used, resource cleanup can easily be done when the execution finishes (connection.close()) as well as destroying a connection if there is an error.

The README mentions that connection pooling support will eventually be added, however, I couldn't find any issue or documentation about this. I apologize if I missed something.

I'm not sure what the plans are for this, but I have some code that uses Javascript Promises and the node-pool module (https://github.com/coopernurse/node-pool) to achieve connection pooling along with (what I hope) is proper closing of connections when executing queries. I could abstract it out and and contribute it to the module if this is something the committers might want to include in this module.

If it's more appropriate for a separate wrapper module, I can see that too, especially since I know not everyone uses Promises.

Also, I didn't see a contributing.md so I hope this is an ok way to engage the community about this.

@ericwaldheim
Copy link
Contributor

@ericwaldheim
Copy link
Contributor

And it is indeed more appropriate for a separate wrapper module.

@theothermattm
Copy link
Author

Thanks @ericwaldheim. I didn't realize that strong-oracle had connection pooling, it's not in the documentation that I could find, but looking through their code I see it now. Have you had success using strong-oracle's pooling? I don't see anything regarding how to configure it.

@theothermattm
Copy link
Author

Also, somewhat related, what is the relationship between strong-oracle and this module, if there is one? I'm assuming they're forks one way or the other, but there doesn't seem to be any reference in either module. All the info I can find on the web suggests that node-oracle is the way to go and strong-oracle is never mentioned.

@cjbj
Copy link

cjbj commented Jan 21, 2015

If we're mentioning other drivers, https://github.com/oracle/node-oracledb also has connection pooling.

@theothermattm
Copy link
Author

@cjbj wow, hot off the presses, I didn't realize oracle was creating their own drivers for node. that's great, I will try it out.

@johannish
Copy link
Collaborator

strong-oracle is a fork of this module (see: https://github.com/strongloop/strong-oracle/blob/84886677557e0aeb8d1eb2bd4c2ebfe88d3a2e4e/NOTICE). Prior to the fork, @raymondfeng was contributing some very helpful commits to this module.

Strong-oracle has been actively maintained by both raymondfeng and bnoordhuis, and is almost certainly more robust than this codebase at this time. The main downside with strong-oracle is its slightly more restrictive (and certainly more complex) license: https://github.com/strongloop/strong-oracle/blob/84886677557e0aeb8d1eb2bd4c2ebfe88d3a2e4e/LICENSE.

Thanks for chiming in, @cjbj! An oracle-backed driver is probably the safest bet, especially given, in my opinion, the complexity of working with Oracle OCCI and its occasionally-lacking documentation.

@cjbj
Copy link

cjbj commented Jan 21, 2015

@theothermattm we did give a heads up to some people, including @joeferner and the StrongLoop crew, and we demoed a very early implementation last year. Yesterday was the big release.

We use the database's C API not the C++ one so there are significant architectural differences underneath.

@theothermattm
Copy link
Author

Wow this is an immensely helpful thread. I'm glad I asked. Thank you @raztus and @cjbj.

@cjbj
Copy link

cjbj commented Jan 21, 2015

@raztus send any OCCI questions my way and I'll make sure to find an answer.

@johannish
Copy link
Collaborator

Also, to address your original question, @theothermattm, about "The README mentions that connection pooling support will eventually be added," I think I put that in because I was planning on merging the connection pooling that raymongfeng was working on in the strong-oracle fork. At the time I think he kept the MIT license from this library, so merging it would have been clearly acceptable (and still might be, going back retroactively to the commits licensed as such.) Now it's less clear.

@cjbj Thanks for the offer! I've all but abandoned coding on this project, though I still follow it and have been meaning to address the outstanding pull requests. With an oracle-sanctioned solution now available, I may not even continue following this.

@theothermattm
Copy link
Author

@raztus it does seem to make sense to move towards the oracle version. Does it make sense to add a note to the README.md for this and merge it into master sooner rather than later?

@theothermattm
Copy link
Author

also, I feel like I should go through all the old stack overflow posts I read to finally arrive at node-oracle and let them know about node-oracledb ;)

@johannish
Copy link
Collaborator

I'll have to leave that up to @joeferner, since this is really his baby, and I've just stepped up to help with some of the maintenance--though I've defaulted on that effort.

Before you start preaching it to the masses, you might consider installing and playing with it first to make sure it fits your needs.

@theothermattm
Copy link
Author

Good point, not so much preaching though as just communicating the news. It was pretty hard for me to find good info on the node.js/oracle topic.

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

No branches or pull requests

4 participants