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

"ConnectionError: Connection is closed." #12

Open
cerebrate opened this Issue Feb 19, 2017 · 15 comments

Comments

Projects
None yet
6 participants
@cerebrate

cerebrate commented Feb 19, 2017

I'm trying to use node-red-contrib-mssql to persist various state in my node-red installation, but while it works great in small-scale manual testing, when I try to use it more fully, most of the queries simply fail, with my debug tab filling up with "ConnectionError: Connection is closed." errors from the various MSSQL nodes.

Is there a known problem here, or an obvious trick to usage/workaround I'm missing?

@wrathagom

This comment has been minimized.

Show comment
Hide comment
@wrathagom

wrathagom Feb 20, 2017

Contributor

It isn't a connection pool, so you need to make sure that the queries are sequential. One query finishes before another one is started.

Without seeing your specific use case my guess is that you start a query, then start another one. Either of those finishes and closes the connection before the 2nd one has the chance to return anything.

Contributor

wrathagom commented Feb 20, 2017

It isn't a connection pool, so you need to make sure that the queries are sequential. One query finishes before another one is started.

Without seeing your specific use case my guess is that you start a query, then start another one. Either of those finishes and closes the connection before the 2nd one has the chance to return anything.

@cerebrate

This comment has been minimized.

Show comment
Hide comment
@cerebrate

cerebrate Feb 20, 2017

...wait, it shares the same connection even across separate flows?

cerebrate commented Feb 20, 2017

...wait, it shares the same connection even across separate flows?

@wrathagom

This comment has been minimized.

Show comment
Hide comment
@wrathagom

wrathagom Feb 20, 2017

Contributor

That's my understanding, assuming you've selected the same connection in the dropdown.

Contributor

wrathagom commented Feb 20, 2017

That's my understanding, assuming you've selected the same connection in the dropdown.

@scdebaay

This comment has been minimized.

Show comment
Hide comment
@scdebaay

scdebaay Apr 18, 2017

After the 0.0.7 update, in which the issue "Use sql.connection.Close()" was solved, this issue reared its' head again. I am using two connections, within one flow, and the nodes are triggered at the same time.
The flow is meant to check the availability of two separate MS SQL instances on the same server.
[ { "id": "473adc4d.2ce10c", "type": "MSSQL", "z": "a7ae457.3c93838", "mssqlCN": "92722840.72ba38", "name": "Test HYDRA", "query": "SELECT * FROM sys.databases WHERE name = 'UMBRACOTEST'", "outField": "payload", "x": 740, "y": 923, "wires": [ [ "20c064ee.be085c" ] ] }, { "id": "66af5bb3.b22b04", "type": "MSSQL", "z": "a7ae457.3c93838", "mssqlCN": "29befeb6.cff9ea", "name": "Test HYDRA\\BE2010", "query": "SELECT * FROM sys.databases WHERE name = 'BEDB'", "outField": "payload", "x": 762, "y": 1002, "wires": [ [ "20c064ee.be085c" ] ] }, { "id": "7cbec159.9589a8", "type": "function", "z": "a7ae457.3c93838", "name": "HYDRA SQL Request", "func": "msg.topic = \"HYDRA\";\nreturn msg;", "outputs": 1, "noerr": 0, "x": 383, "y": 922, "wires": [ [ "473adc4d.2ce10c" ] ] }, { "id": "ee03803.4bbcf8", "type": "inject", "z": "a7ae457.3c93838", "name": "Start Polling SQL", "topic": "", "payload": "", "payloadType": "date", "repeat": "60", "crontab": "", "once": true, "x": 109, "y": 960, "wires": [ [ "7cbec159.9589a8", "28e1c672.ab9342" ] ] }, { "id": "28e1c672.ab9342", "type": "function", "z": "a7ae457.3c93838", "name": "HYDRA\\BE2010 Request", "func": "msg.topic = \"HYDRA\\\\BE2010\";\nreturn msg;", "outputs": 1, "noerr": 0, "x": 392, "y": 1001, "wires": [ [ "66af5bb3.b22b04" ] ] }, { "id": "92722840.72ba38", "type": "MSSQL-CN", "z": "", "name": "HYDRA", "server": "HYDRA.de-baay.nl", "encyption": true, "database": "master" }, { "id": "29befeb6.cff9ea", "type": "MSSQL-CN", "z": "", "name": "HYDRA BE2010", "server": "HYDRA\\BE2010", "encyption": true, "database": "master" } ]

Apparently it is possible to use pooling, according to this post:
tediousjs/node-mssql#164

Why is this not implemented in this node by default. Nowadays asynchronous transactions are much more usual than before, or am I looking at this way too easy?

scdebaay commented Apr 18, 2017

After the 0.0.7 update, in which the issue "Use sql.connection.Close()" was solved, this issue reared its' head again. I am using two connections, within one flow, and the nodes are triggered at the same time.
The flow is meant to check the availability of two separate MS SQL instances on the same server.
[ { "id": "473adc4d.2ce10c", "type": "MSSQL", "z": "a7ae457.3c93838", "mssqlCN": "92722840.72ba38", "name": "Test HYDRA", "query": "SELECT * FROM sys.databases WHERE name = 'UMBRACOTEST'", "outField": "payload", "x": 740, "y": 923, "wires": [ [ "20c064ee.be085c" ] ] }, { "id": "66af5bb3.b22b04", "type": "MSSQL", "z": "a7ae457.3c93838", "mssqlCN": "29befeb6.cff9ea", "name": "Test HYDRA\\BE2010", "query": "SELECT * FROM sys.databases WHERE name = 'BEDB'", "outField": "payload", "x": 762, "y": 1002, "wires": [ [ "20c064ee.be085c" ] ] }, { "id": "7cbec159.9589a8", "type": "function", "z": "a7ae457.3c93838", "name": "HYDRA SQL Request", "func": "msg.topic = \"HYDRA\";\nreturn msg;", "outputs": 1, "noerr": 0, "x": 383, "y": 922, "wires": [ [ "473adc4d.2ce10c" ] ] }, { "id": "ee03803.4bbcf8", "type": "inject", "z": "a7ae457.3c93838", "name": "Start Polling SQL", "topic": "", "payload": "", "payloadType": "date", "repeat": "60", "crontab": "", "once": true, "x": 109, "y": 960, "wires": [ [ "7cbec159.9589a8", "28e1c672.ab9342" ] ] }, { "id": "28e1c672.ab9342", "type": "function", "z": "a7ae457.3c93838", "name": "HYDRA\\BE2010 Request", "func": "msg.topic = \"HYDRA\\\\BE2010\";\nreturn msg;", "outputs": 1, "noerr": 0, "x": 392, "y": 1001, "wires": [ [ "66af5bb3.b22b04" ] ] }, { "id": "92722840.72ba38", "type": "MSSQL-CN", "z": "", "name": "HYDRA", "server": "HYDRA.de-baay.nl", "encyption": true, "database": "master" }, { "id": "29befeb6.cff9ea", "type": "MSSQL-CN", "z": "", "name": "HYDRA BE2010", "server": "HYDRA\\BE2010", "encyption": true, "database": "master" } ]

Apparently it is possible to use pooling, according to this post:
tediousjs/node-mssql#164

Why is this not implemented in this node by default. Nowadays asynchronous transactions are much more usual than before, or am I looking at this way too easy?

@wrathagom

This comment has been minimized.

Show comment
Hide comment
@wrathagom

wrathagom Apr 18, 2017

Contributor

I'm not the original creator, but my guess is that when this repo was created it was before node-mssql had pools and it's just not been updated since then. I am fairly certain that there is another package https://www.npmjs.com/package/node-red-contrib-mssql-pool that implements it as a pool. @fitz0019 can you look into pulling this one in? It would likely have to be a merge, but looks like good stuff.

Contributor

wrathagom commented Apr 18, 2017

I'm not the original creator, but my guess is that when this repo was created it was before node-mssql had pools and it's just not been updated since then. I am fairly certain that there is another package https://www.npmjs.com/package/node-red-contrib-mssql-pool that implements it as a pool. @fitz0019 can you look into pulling this one in? It would likely have to be a merge, but looks like good stuff.

@tomko80

This comment has been minimized.

Show comment
Hide comment
@tomko80

tomko80 Aug 15, 2017

Having this problem when wanting to store the payload produced by a splitter node iterating an array.
Tried also with a custom loop following this example: https://flows.nodered.org/flow/47323c946caa06cf3c7e

Seems the MSSQL node works fine alone but even the simplest select statement will fail if called in a loop. Can you please find a fix for this? Not so uncommon one needs to push an array into rows.

tomko80 commented Aug 15, 2017

Having this problem when wanting to store the payload produced by a splitter node iterating an array.
Tried also with a custom loop following this example: https://flows.nodered.org/flow/47323c946caa06cf3c7e

Seems the MSSQL node works fine alone but even the simplest select statement will fail if called in a loop. Can you please find a fix for this? Not so uncommon one needs to push an array into rows.

@tomko80

This comment has been minimized.

Show comment
Hide comment
@tomko80

tomko80 Aug 15, 2017

@wrathagom , the pool version seems also to fail. Throwing the error "Error: Global connection already exists. Call sql.close() first." instead.

tomko80 commented Aug 15, 2017

@wrathagom , the pool version seems also to fail. Throwing the error "Error: Global connection already exists. Call sql.close() first." instead.

@wrathagom

This comment has been minimized.

Show comment
Hide comment
@wrathagom

wrathagom Aug 15, 2017

Contributor

Just checking, did you have your pool size set appropriately high?

Contributor

wrathagom commented Aug 15, 2017

Just checking, did you have your pool size set appropriately high?

@wrathagom

This comment has been minimized.

Show comment
Hide comment
@wrathagom

wrathagom Aug 15, 2017

Contributor

also, are you isolating these? I doubt you can have more than one installed at the same time.

Contributor

wrathagom commented Aug 15, 2017

also, are you isolating these? I doubt you can have more than one installed at the same time.

@tomko80

This comment has been minimized.

Show comment
Hide comment
@tomko80

tomko80 Aug 15, 2017

I used the default pool size of 10, the my test only iterates over 3 items.

Just tried out the node-red-mssql-node and it seems to suffer from the same problem. Can this be in the base mssql lib? There has been some discussions... tediousjs/node-mssql#138

tomko80 commented Aug 15, 2017

I used the default pool size of 10, the my test only iterates over 3 items.

Just tried out the node-red-mssql-node and it seems to suffer from the same problem. Can this be in the base mssql lib? There has been some discussions... tediousjs/node-mssql#138

@wrathagom

This comment has been minimized.

Show comment
Hide comment
@wrathagom

wrathagom Aug 15, 2017

Contributor

I'll have some time to test it myself here in a bit.

Contributor

wrathagom commented Aug 15, 2017

I'll have some time to test it myself here in a bit.

@tomko80

This comment has been minimized.

Show comment
Hide comment
@tomko80

tomko80 Aug 18, 2017

@wrathagom , could you reproduce the problem/locate the fault?

tomko80 commented Aug 18, 2017

@wrathagom , could you reproduce the problem/locate the fault?

@kasarol

This comment has been minimized.

Show comment
Hide comment
@kasarol

kasarol Feb 5, 2018

Any updates on this? I'm getting the same error, with and without using the connection pool variant (node-red-contrib-mssql-pool). Very easy to reproduce by copy pasting any basic query to 5 parallel paths. Randomly 0-5 of them fail to this issue. And it's not even possible to create any retry system due to no error catching from the node.

kasarol commented Feb 5, 2018

Any updates on this? I'm getting the same error, with and without using the connection pool variant (node-red-contrib-mssql-pool). Very easy to reproduce by copy pasting any basic query to 5 parallel paths. Randomly 0-5 of them fail to this issue. And it's not even possible to create any retry system due to no error catching from the node.

@iba-jsykes

This comment has been minimized.

Show comment
Hide comment
@iba-jsykes

iba-jsykes Feb 5, 2018

Contributor

If you change your package.json to use the latest node-mssql and tedious, you can try out the async/await example in place of the current promises based implementation. Thats if you feel comfortable creating a custom node of course. Definitely worked for me.

Contributor

iba-jsykes commented Feb 5, 2018

If you change your package.json to use the latest node-mssql and tedious, you can try out the async/await example in place of the current promises based implementation. Thats if you feel comfortable creating a custom node of course. Definitely worked for me.

@kasarol

This comment has been minimized.

Show comment
Hide comment
@kasarol

kasarol Feb 6, 2018

I made a pull request with a fix for this issue.

kasarol commented Feb 6, 2018

I made a pull request with a fix for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment