MASSIVE performance increase of SQL data source #119

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants
  • Runs SQL queries in a separate thread every 5 ticks. Prevents connections to database blocking the main thread.
  • Store references to TownBlocks within the TownBlocks table rather than in a giant string

Tested working.

Note that code has not been added to automatically update a users database.

ShadowDog007 added some commits Feb 4, 2014

@ShadowDog007 ShadowDog007 Run SQL queries asynchronously
Running SQL queries in the main thread is not a good idea because they
can take quite a while at times.
1f26bc8
@ShadowDog007 ShadowDog007 Reduced size of TownBlock coords in SQL
BIGINT is 64bits. Chunk coordinates are 32bit ints.

mediumint is more than big enough for referencing chunks. Range of
+-8388607
65d5273
@ShadowDog007 ShadowDog007 Store refs to TownBlocks in the TownBlock table
Storing references to townblocks in a massive string is a HUGE waste of
time. With large towns it freezes the main thread for very long periods
of time when they are required to update. (Just from building that
string)

Instead, storing a reference to the town each town block belongs to
inside the townblocks table allows updating and loading of town blocks
to occur without issue.
29385ea
Owner

ElgarL commented Feb 6, 2014

This is a nice piece of code, however.
It doesn't handle DB conversion so it breaks all older databases.
It doesn't handle reload/restart so saving the queues will allow it to resume but it won't handle a restart.

Well fix it yourself then.

It was ridiculous what you were doing before. My friend had a 200KB string in his towny table.

Owner

ElgarL commented Feb 7, 2014

The SQL implementation was simply a flat-file port. It has never been optimised.
I'm commenting to say we don't accept partial fixes as pulls.

Well fix it yourself then.

It took me like 20 minutes to write that. You should have done it right in the first place.

Owner

ElgarL commented Feb 7, 2014

Don't bring an attiutde here.

One more thing to note, not sure if plotPrice is handled correctly if it is taken from the town.

ErgoAsh commented Feb 24, 2014

Is that fix is fully working now without changing something in database?

Owner

ElgarL commented Feb 24, 2014

No it's not backwardly compatible, but we will incorporate this once time allows.

ErgoAsh commented Feb 24, 2014

Thanks for response, I'm waiting.

@ErgoAsh ErgoAsh commented on the diff Feb 24, 2014

src/com/palmergames/bukkit/towny/db/TownyDataSource.java
@@ -97,6 +97,9 @@ public boolean saveAll() {
return saveWorldList() && saveNationList() && saveTownList() && saveResidentList() && saveWorlds() && saveNations() && saveTowns() && saveResidents() && saveAllTownBlocks() && saveRegenList() && saveSnapshotList();
}
+
+ public boolean shutdown() {
@ErgoAsh

ErgoAsh Feb 24, 2014

I do not know much about databases but whether this method is properly constructed?
My Eclipse returns "This method must return a result of type boolean" and I can add return for firstRun of false.

@ShadowDog007

ShadowDog007 Feb 24, 2014

Yeah, thats my bad, I forgot to add return true =P

@ShadowDog007

ShadowDog007 Feb 24, 2014

Just working on a few other things atm.

I don't think its 100% safe to use, mate is starting to have some issues and players are losing some of their town blocks. Need to figure out why.

@ErgoAsh

ErgoAsh Feb 25, 2014

Thanks for response, those fix and boost are important for me, I'll setup PHPMyAdmin and MySQL on my computer then I'll try test that.

@ShadowDog007

ShadowDog007 Feb 25, 2014

If your going to test....

My mate told me this. It works fine, but it seems plots are lost after restarting. Also 'Possible temp fix for new towns who're claiming land- Put the plot forsale at 0 and claim it as your personal land, the chunks that are owned do not disappear'

When I was testing it all plots saved to database, so not sure if some of them are not being saved or they are being deleted for some reason.

Owner

ElgarL commented Apr 8, 2014

Included most of this with my update to use prepared statements.

ElgarL closed this Apr 8, 2014

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