search

whimxiqal / journey

A server-side path-finding Minecraft plugin
MIT License
14 stars 3 forks source link

Enabled MySQL code #38

Closed TheDeafCreeper closed 1 year ago

TheDeafCreeper commented 1 year ago

Resolves #30. Currently non functional it seems as it's not creating one or more tables.

TheDeafCreeper commented 1 year ago

Alright so there seems to be three main issues right now:

  1. The tables are not being automatically created for some reason.
  2. DB access does not seem to be async, so it causes lag whenever it read/writes.
  3. Timestamps seem to be wrong (at least for adding waypoints)
TheDeafCreeper commented 1 year ago

I suppose issue 2 is what #33 is for.

TheDeafCreeper commented 1 year ago

Now that I think about it, I almost wonder if #37 is caused by DB access as well, just significantly less noticeable due to being a SQLite DB currently.

Edit: After some testing it didn't seem like the case, but when looking at the spark timing in the issue it looks like it is a call to SqlPathRecordManager causing the lag.

TheDeafCreeper commented 1 year ago

Alright, cause for issue 1 is the fact journeydb.ver exists, which it almost always will when switching to MySQL.

TheDeafCreeper commented 1 year ago

Issues 1 and 3 should be fixed now, leaving performance issues.

whimxiqal commented 1 year ago

I didn't use if not exists because not all database engines has that technology, so I'm managing database version manually. Although, I see the problem with have a single version file separate from the database itself rather than storing the version in the database itself because changing database engine will use the old database version file.

The whole versioning system exists to handle future database migrations (schema alterations) which is bound to happen.

To fix this, we should create a new version management system. I'm leaning towards just creating another table called journey_db_version that just has a single indexed integer column. MAX() of that column will dictate what version the database is on, and the db migrations for every missing version will be applied and the updated version will be set on that table.

For backwards compatibility, we'll have to check if there's a .ver file, which would insinuate the db is already on version 1, and since we'll be enabling MySQL at the same time as the new versioning method, it must be version 1 on SQLite. So, from there we'll update to the current version by applying the migrations needed (in this case, we have a migration to add a version table) and then set the version properly. We can also delete the .ver at that point.

TheDeafCreeper commented 1 year ago

Fair enough on the if not exists.

I'll write up something to still check DB version, just in the DB itself

TheDeafCreeper commented 1 year ago

Alright I think this should all be functional now.

TheDeafCreeper commented 1 year ago

Actually just removed the legacy functions since if they're needed again they can be fetched via git.

TheDeafCreeper commented 1 year ago

The lag with MySQL probably won't matter on a local DB, though if it does then we would probably want to make connection pooling part of this PR.

Edit: Can confirm, it does not seem to cause any more lag than with SQLite when it's on the same machine.

TheDeafCreeper commented 1 year ago

Alright I think all the changes should be made, and the plugin works in the testing I did.

TheDeafCreeper commented 1 year ago

Also I changed Intelij settings so imports should hopefully stop being combined.

TheDeafCreeper commented 1 year ago

I realized I forgot to actually test the latest changes, and apparently it doesn't actually work because the connection just closes immediately after it's used it seems.

TheDeafCreeper commented 1 year ago

Alright, fixed the connection issue.

TheDeafCreeper commented 1 year ago

That should be everything resolved, now to: