Skript icon indicating copy to clipboard operation
Skript copied to clipboard

Get SQL databases working again

Open TheLimeGlass opened this issue 11 months ago • 20 comments

Description

  • Adds H2 as a configurable database. Will probably make default eventually or maybe in this pull request?
  • Removes SQLibrary and adds https://github.com/brettwooldridge/HikariCP as the main SQL wrapper.
  • Revamped API for better registration and handling of JDBC Java drivers.
  • Added a commit changes option which is essentially how Skript used to operate. Changed the JDBC standard to auto commit after every edit (https://github.com/SkriptLang/Skript/issues/2007). This is standard and better for performance. Skript didn't for some reason. I speculate it was to allow MySQL to sync with other servers, so either way it's an option now.

Perks

  • Better configuration options with HikariCP.
  • Way better performance with HikariCP.
  • Easier implementation and not relying on a small project like SQLibary.
  • Thread pooling potential with HikariCP.
  • This new API design allows for quite literally any JDBC implementation database.
  • Uses the SpigotLibraryLoader to load the resources at runtime rather than shadowing into the jar.

TODO

  • [x] Test MySQL.
  • [x] Test SQLite.
  • [x] Test H2.
  • [x] Run beta testing in the SkriptLang Discord.
  • [x] Adds tests.
  • [x] In H2 use H2's MERGE INTO instead of INSERT.
  • [x] Forgot to add a check that HikariCP classes were loaded by SpigotLibraryLoader.
  • [x] Rename SQLStorage to something with JDBC reference, maybe JdbcStorage as it's not only SQL anymore.
  • [ ] Test that monitor changes still do something.
  • [x] Attach a SkriptAddon instance to the registration, so Skript knows where a storage is coming from.

Optional

  • [ ] Write up an API tutorial for registering custom storage types.
  • [ ] Document all the possible options for databases or have annotations for a documentation system.
  • [x] See if Skript could handle async and multithreading now that the database has the possibility.
  • [ ] Maybe add MongoDB support https://github.com/SkriptLang/Skript/issues/1787
  • [ ] Change the existing behavior of dumping every variable into ram on server start.
  • [ ] Investigate if this solves https://github.com/SkriptLang/Skript/issues/2022

Notes:

  • Should we just remove SQLite support? It's not like anyone is using it right now, it doesn't work currently.
  • H2 is a flat file SQL implementation that supports asynchronous operations and multi threaded unlike SQLite in both those regards.
  • This does not impact anything to do with the default flat file CSV.
  • This mainly only affects JDBC driver databases. The rework is still a viable experiment started by bensku https://github.com/SkriptLang/Skript/tree/feature/variables2

Testing and using this jar

To test this experimental feature out, go to the "checks" tab and then click a Java version on the side and then click the nightly artifacts to get a built jar of the latest commit.


Target Minecraft Versions: any Requirements: none Related Issues: https://github.com/SkriptLang/Skript/issues/1168, https://github.com/SkriptLang/Skript/issues/2007, https://github.com/SkriptLang/Skript/issues/1478 and https://github.com/SkriptLang/Skript/issues/3948

TheLimeGlass avatar Feb 27 '25 04:02 TheLimeGlass

could you push the SkriptLang branch to your fork, then do the work there? once things are ready you can open a PR from that branch to SkriptLang

Pikachu920 avatar Feb 27 '25 05:02 Pikachu920

It is on my branch. Close the old one, change target and transfer the description to this one if that's what you want.

TheLimeGlass avatar Feb 27 '25 05:02 TheLimeGlass

I guess I'm confused why we'd merge into enhancement/sql instead of having your work on your fork's branch and then you can open a PR from the fork branch to dev/feature

Pikachu920 avatar Feb 27 '25 05:02 Pikachu920

Because I don't have access

TheLimeGlass avatar Feb 27 '25 11:02 TheLimeGlass

@Pikachu920 reopen

TheLimeGlass avatar Mar 10 '25 17:03 TheLimeGlass

Hello, I just want to report that synchronization works great besides 1 thing If you remove variable from 1 server (it now displays as <none> on that server where you executed a deletion), all other servers that share this variable/mysql won't be affected and they will keep their last known variable (i assume the variable when monitor changes executes is stored in server's cache/memory), so by that it does not get synchronized properly, it should also become <none> on all servers that monitor same databases

I have been testing skript mysql implementation for more than 4 months now, not constantly, but very actively, i think this is the only issue left

I also would like to ask how optimized is the monitor-changes functionality for 2 reasons (2 questions!):

  1. If we have 2+ or even 5+ mysql databases connected on the same network of sub-servers, what should lowest optimal monitor-changes delay be? What about 0.5? Just curious
  2. What is the way of working for monitor-changes and commiting changes? Like, let's say that 2 different servers connected on same skript database try to set {synced_example::test1} and {synced_example::test2} to some value, will the first server who tries to commit changes "lock" the database and prevent second one from inserting this variable (if the timing for both servers commiting is very similar)? I am also interested to know the same thing for monitor-changes - what if multiple servers try to look up / check for changes on same database, will the first server who tries to check for changes "lock" database and make other servers fail to pickup latest changes from database?

These questions are based on my testings of plugin's this new SQL implementation

MotikaCraft-TheJoshua avatar Mar 19 '25 11:03 MotikaCraft-TheJoshua

  1. Set some variable on server1 to some value
  2. Show those variables on server2 - the values will show (after defined delay of monitor-changes)
  3. Delete those variables on server2
  4. So because of the way currently the system works (if you delete variable (making it unset) on any server that synchronizes variables, that won't be changed/synchronized to all other servers sharing the same database), the server2 will now have <none> value for those variables, even tho all other synchronized sub-servers will keep last known value and will not have it unset

So now the value of variable is removed from MySQL and all other servers should sync with to that, however, what happens next is that if you restart server2, it loads up, the variables are still <none> but now if you use server1 to set those variables to something else, server2 will never again synchronize variables, even if you restart it, even if after restart you use server2 to change variable to something else, it won't sync anywhere on network

It also will not sync any other databases for some reason that are shared also by the network sub-servers... this gets so weird, i probably should have tested this lol. Any database i had connected now cannot monitor changes (maybe it does, but there is no info about it) even after i restart them, i don't know what happened.

MotikaCraft-TheJoshua avatar Mar 19 '25 12:03 MotikaCraft-TheJoshua

  1. Set some variable on server1 to some value
  2. Show those variables on server2 - the values will show (after defined delay of monitor-changes)
  3. Delete those variables on server2
  4. So because of the way currently the system works (if you delete variable (making it unset) on any server that synchronizes variables, that won't be changed/synchronized to all other servers sharing the same database), the server2 will now have <none> value for those variables, even tho all other synchronized sub-servers will keep last known value and will not have it unset

So now the value of variable is removed from MySQL and all other servers should sync with to that, however, what happens next is that if you restart server2, it loads up, the variables are still <none> but now if you use server1 to set those variables to something else, server2 will never again synchronize variables, even if you restart it, even if after restart you use server2 to change variable to something else, it won't sync anywhere on network

It also will not sync any other databases for some reason that are shared also by the network sub-servers... this gets so weird, i probably should have tested this lol. Any database i had connected now cannot monitor changes (maybe it does, but there is no info about it) even after i restart them, i don't know what happened.

Hello, I've just read your 2 messages (above) and I can tell you that you're absolutely right, I reported this problem just over 2 months ago on the official new SQL test discord.

I have a question, you don't talk about auto-commit feature, did you try to disable it by setting it to disabled or enable it on enabled, do you have an error, because I had tested a few weeks ago and I still had the error.

Normally if you don't use the auto-commit feature, it's normal that it doesn't synchronize normally because auto-commit allows you to modify the state of the skript variables on each server by synchronizing them etc...

But since I started using this feature, I still can't get auto-commit to work because there are one or more errors in the console. Anyway, I just wanted to let you know if you had the same problem as me. Have a nice day!

I want to give you an example of an error I had with the feature I'm talking about: https://pastebin.com/7J5Frbzf

Lennord avatar Mar 19 '25 12:03 Lennord

  1. Set some variable on server1 to some value
  2. Show those variables on server2 - the values will show (after defined delay of monitor-changes)
  3. Delete those variables on server2
  4. So because of the way currently the system works (if you delete variable (making it unset) on any server that synchronizes variables, that won't be changed/synchronized to all other servers sharing the same database), the server2 will now have <none> value for those variables, even tho all other synchronized sub-servers will keep last known value and will not have it unset

So now the value of variable is removed from MySQL and all other servers should sync with to that, however, what happens next is that if you restart server2, it loads up, the variables are still <none> but now if you use server1 to set those variables to something else, server2 will never again synchronize variables, even if you restart it, even if after restart you use server2 to change variable to something else, it won't sync anywhere on network

It also will not sync any other databases for some reason that are shared also by the network sub-servers... this gets so weird, i probably should have tested this lol. Any database i had connected now cannot monitor changes (maybe it does, but there is no info about it) even after i restart them, i don't know what happened.

Even if this PR is merged, using the same variables database across multiple severs wouldn't be supported.

Pikachu920 avatar Mar 19 '25 15:03 Pikachu920

I have a question, you don't talk about auto-commit feature, did you try to disable it by setting it to disabled or enable it on enabled, do you have an error, because I had tested a few weeks ago and I still had the error.

Normally if you don't use the auto-commit feature, it's normal that it doesn't synchronize normally because auto-commit allows you to modify the state of the skript variables on each server by synchronizing them etc...

But since I started using this feature, I still can't get auto-commit to work because there are one or more errors in the console. Anyway, I just wanted to let you know if you had the same problem as me. Have a nice day!

I want to give you an example of an error I had with the feature I'm talking about: https://pastebin.com/7J5Frbzf

I did not touch auto commit option, and i think you mean commit changes and not auto-commit? I do not have this option, maybe my Skript build is outdated, later today or this week i will update to very latest and check if this option exist there, i only have this

	#commit changes: 0.5 seconds
	# If you want to change how frequently SQL changes are commited.
	# If this is disabled, auto commit will be enabled.
	# Pros to this would be on external databases such as MySQL where it gives other servers time to react to changes.

this is commented out for mysql example, so i assume it uses that auto commit feature, because it says "if this is disabled..." and commit changes is disabled = it's commented

Even if this PR is merged, using the same variables database across multiple severs wouldn't be supported.

Ok wow, well i did not know that, i thought that was the purpose of having MySQL implementation in Skript Is it even planned to support that? i mean i see to some extent it works but i am only testing this, was having hopes to use that both for local (subserver/backend only) & global (network/cross-server supported) variables

MotikaCraft-TheJoshua avatar Mar 19 '25 15:03 MotikaCraft-TheJoshua

I have a question, you don't talk about auto-commit feature, did you try to disable it by setting it to disabled or enable it on enabled, do you have an error, because I had tested a few weeks ago and I still had the error. Normally if you don't use the auto-commit feature, it's normal that it doesn't synchronize normally because auto-commit allows you to modify the state of the skript variables on each server by synchronizing them etc... But since I started using this feature, I still can't get auto-commit to work because there are one or more errors in the console. Anyway, I just wanted to let you know if you had the same problem as me. Have a nice day! I want to give you an example of an error I had with the feature I'm talking about: https://pastebin.com/7J5Frbzf

I did not touch auto commit option, and i think you mean commit changes and not auto-commit? I do not have this option, maybe my Skript build is outdated, later today or this week i will update to very latest and check if this option exist there, i only have this

	#commit changes: 0.5 seconds
	# If you want to change how frequently SQL changes are commited.
	# If this is disabled, auto commit will be enabled.
	# Pros to this would be on external databases such as MySQL where it gives other servers time to react to changes.

this is commented out for mysql example, so i assume it uses that auto commit feature, because it says "if this is disabled..." and commit changes is disabled = it's commented

Yes, I was talking about the commit changes feature, sorry for getting the name of the feature wrong. Saw the message above (from Pikachu920). I hope that one day we'll be able to synchronize variables on several servers, which unfortunately won't be the case for now :(.

Lennord avatar Mar 19 '25 17:03 Lennord

Yes, I was talking about the commit changes feature, sorry for getting the name of the feature wrong. Saw the message above (from Pikachu920). I hope that one day we'll be able to synchronize variables on several servers, which unfortunately won't be the case for now :(.

It would be really incredible to have that, i mean it is kinda working at the moment, for now you need some kind of a bridge plugin which you can utilize inside of skript itself, so basically only selection of variables can be used cross-server Still, can't wait if this ever happens

MotikaCraft-TheJoshua avatar Mar 19 '25 18:03 MotikaCraft-TheJoshua

Closing due to change requests outstanding for 6 months (https://github.com/SkriptLang/Skript/pull/5646#pullrequestreview-2509968914)

sovdeeth avatar Jun 03 '25 05:06 sovdeeth

huh?

TheLimeGlass avatar Jun 03 '25 17:06 TheLimeGlass

Get Pickle to write the new requested changes. Open this pull request

TheLimeGlass avatar Jun 03 '25 17:06 TheLimeGlass

Capture

Hello, we're reporting a major issue with the "commit-changes" option (via Discord testing sql-databases).

When you enable the option indicated above, it throws an error that the database is not properly connected. I'm informing you, based on several tests, that it's not the database or the machine hosting it that's causing this issue, but rather the new SQL system.

Despite several tests, I wanted to know if this issue can be fixed as quickly as possible because without this option, synchronization is impossible, as shown in the image.

Good evening!

Lennord avatar Sep 06 '25 17:09 Lennord

@TheLimeGlass Any ideas about the above issue? I've seen it reported a few times now, not sure what's up there.

sovdeeth avatar Sep 18 '25 01:09 sovdeeth

Capture Hello, we're reporting a major issue with the "commit-changes" option (via Discord testing sql-databases).

When you enable the option indicated above, it throws an error that the database is not properly connected. I'm informing you, based on several tests, that it's not the database or the machine hosting it that's causing this issue, but rather the new SQL system.

Despite several tests, I wanted to know if this issue can be fixed as quickly as possible because without this option, synchronization is impossible, as shown in the image.

Good evening!

Good evening @TheLimeGlass, can you please check this issue as soon as possible?

Lennord avatar Oct 02 '25 17:10 Lennord

Hello, I think we need to redo the "commit-changes" option system, what do you think?

Lennord avatar Nov 03 '25 16:11 Lennord

@UnderscoreTud and @APickledWalrus Hello, can you analyze the code due to the review request?

WynoriaStudios avatar Nov 05 '25 16:11 WynoriaStudios