btk5h / skript-db

MIT License
5 stars 5 forks source link

mysql not closing out connections #2

Closed wokka1 closed 6 years ago

wokka1 commented 6 years ago

I'm having the same issue as pepper82 and the others, when this skript gets loaded: https://gist.github.com/wokka1/b83a1494d5dc466d96bde204d5e4dad4 it creates 10 connections to mysql, from looking at show processlist at the console. If I reload the skript after an edit, it doesn't close those 10, but opens 10 more. They do not seem to close until you stop the minecraft server.

It's possible I have something in my code wrong, I'm not a programmer.

Also, my select statement works fine in the mysql console, but I'm getting " set Invalid argument value: java.io.NotSerializableException to last database error" output via the last sql error statement.

This is running paperclip b1264, latest skript dev32c, skRayFall and skUtilities addons, plus several spigot/bukkit plugins. I can give you that list if needed for troubleshooting.

btk5h commented 6 years ago

I think I understand what's going on. Whenever you reload the script, it replaces {sql} with a new set of connection credentials. I'll fix this next update to reuse any existing connections, but for now, you can add a {sql} is not set check before setting {sql}

wokka1 commented 6 years ago

Thanks, will test that, any thoughts as to the 2nd error?

wokka1 commented 6 years ago

This does resolve the issue where it opens 10 connections on every refresh. I worked out my 2nd error, was an interpretation of the sql statement. I was putting single ' around the variable and I shouldn't.

Speaking of, why does it open 10 connections to the database? It's not an issue currently, but if I have multiple Skripts connecting to different database, this could add up. Doubt it would really cause a problem, but more curious than anything.

Thanks!

btk5h commented 6 years ago

The connection pool that skript-db uses initializes a bunch of connections to ensure that there is always a connection available to fulfill requests. Since creating database connections can be expensive, skript-db initializes them as soon as possible to stay performant even if there is a spike in requests.

That being said, I do realize that this could cause issues with databases that restrict the number of active connections, so I may make this configurable in the future.

melinstagibson commented 6 years ago

so i guess i can also use {sql} is set in all the skripts and only need to set it once? If this isn't going to be fixed a mention in the wiki would help...

styxnyx commented 6 years ago

I think I understand what's going on. Whenever you reload the script, it replaces {sql} with a new set of connection credentials. I'll fix this next update to reuse any existing connections, but for now, you can add a {sql} is not set check before setting {sql}

This has helped me a ton, I was sitting here going why is this thing keep kicking me out. 👍

bloggy commented 6 years ago

For me this is not fixed, yet. Still "too many connections" issue.

btk5h commented 6 years ago

This issue should be fixed, a release with the fix just hasn't been created yet.