-
-
Notifications
You must be signed in to change notification settings - Fork 390
Get SQL databases working again #7653
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
base: dev/feature
Are you sure you want to change the base?
Get SQL databases working again #7653
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See existing change requests from pickle at previous pr, too: #5646 (review)
### VS Code ### | ||
.vscode/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't be in this pr
src/main/java/org/skriptlang/skript/variables/storage/H2Storage.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/variables/storage/MySQLStorage.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/variables/storage/SQLiteStorage.java
Outdated
Show resolved
Hide resolved
src/test/java/org/skriptlang/skript/variables/storage/H2StorageTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/skriptlang/skript/variables/storage/SQLiteStorageTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes from #5646 not addressed
Hello, I just want to report that synchronization works great besides 1 thing 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
These questions are based on my testings of plugin's this new SQL implementation |
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 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 Normally if you don't use the But since I started using this feature, I still can't get I want to give you an example of an error I had with the feature I'm talking about: https://pastebin.com/7J5Frbzf |
Even if this PR is merged, using the same variables database across multiple severs wouldn't be supported. |
I did not touch auto commit option, and i think you mean
this is commented out for mysql example, so i assume it uses that auto commit feature, because it says "if this is disabled..." and
Ok wow, well i did not know that, i thought that was the purpose of having MySQL implementation in Skript |
Yes, I was talking about the |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes from @APickledWalrus in #5646 still not addressed
Description
commit changes
option which is essentially how Skript used to operate. Changed the JDBC standard to auto commit after every edit (Make the 5-min variable saving period configurable #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
TODO
MERGE INTO
instead ofINSERT
.Optional
Notes:
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: #1168, #2007, #1478 and #3948