Separate out action key index from location to speed up lookups #201
Conversation
| st.executeUpdate(query); | ||
|
|
||
| // Re-create location index | ||
| query = "ALTER TABLE `" + prefix + "data` CREATE INDEX `location` (`world_id`, `x`, `z`, `y`)"; |
There was a problem hiding this comment.
I know it's the only way to do this, but this might need to come with a hefty warning when it's released. One of our servers we has 55M rows in the data table and will likely halt startup for quite some time. An unexpected long startup time could cause someone to abort/kill the server and start it again, without knowing this is intentional. Perhaps this should come with a big console warning?
This is all called in onEnable() (and on main thread) but if I recall correctly, this will all happen before the main thread watchdog starts.
There was a problem hiding this comment.
Oh, actually I was wrong, it's not called in-line within onEnable():
onEnable()called during on start up- An "async" task is launched to establish DB connections.
- This async task also launches a "sync" task (called
enabled()) to perform upgrades.
After reading this, now I am a little concerned about potential start up issues with the Watchdog restarting the server. This will need testing on a large database.. or you can just put a 300s sleep in the v8_to_v9() method to simulate a long upgrade time.
There was a problem hiding this comment.
I believe you make some really valid points in terms of largers servers. It might be prudent to start Prism with the recorder paused, run the upgrader async in the background then have the upgrader unpause the recorder once it's done. Might require a new PR for it.
There was a problem hiding this comment.
I can look at moving the updater async...although I thought it already was
Db startup is definitely already handled async
|
Issues
======
- Added 1
See the complete overview on Codacy |
|
What's the status of this PR? Is this abandoned? |
|
It needs testing on a large dataset until someone has done that and provided the stats its stagnant |
Sounds good. I'll jump on this soon 👍 |
|
This repository is no longer active. You can try to resubmit this at the current Prism repo here https://github.com/prism/PrismRefracted (although, some of these older fix PRs were already adapted, please verify first) |
Hello there!
Yet another PR from me with some SQL index voodoo magick to help speedup lookups! Like my first PR #194, I am highly confident this PR will only bring about sunshines and rainbows to the performance of Prism.
In a nutshell, there is no reason for the
action_idcolumn to be in the same index as thelocation-based columns. By separating it out into it's own index, the performance gain is exponential. A simple command like/pr lookup a:command t:1y r:globalcan take about 5 seconds on my server, as compared to more than 15 minutes (before I killed it) without this indexing key!Unfortunately, because of the way the database engine works, there is no way to modify an existing index, so I tried to take the approach of deleting the whole index
locationand re-creating it with the correctlocation-y columns. This may be the most expensive part about this PR, but I believe this change makes the most logical sense and should be carried out!