search

sammypanda / MCJE-PlayerQuests-Plugin

Survival-friendly questing by the players for the players!
GNU General Public License v3.0
4 stars 0 forks source link

Fix concurrency issues with database #67

Closed sammypanda closed 2 months ago

sammypanda commented 2 months ago

Maybe just make a boilerplate private func in Database that all the methods use which, at least:

Considering using a db engine that can do concurrency and won't lock when writing. Aka: migrating away from sqlite :( ~ i guess it's time i become one of the cool kids using a big chonky db engine instead. maybe postgres, maybe mongo, probably whichever one fits in nicest with the existing Database class; so probably postgres.

Going to do data caches and all that real-time data work in classes. We'll use Database for preservation of state rather than for real-time data.

sammypanda commented 2 months ago

I can't say how exactly to implement, but it would be great if everything was 'able' to wait for data from the db/filesystem/etc.

Alternatively, and frankly far better, is just getting a db solution that can do concurrent writes.

sammypanda commented 2 months ago

Won't do a db solution that does concurrent writes, going to use a flat file because i value a simple setup.

sammypanda commented 2 months ago

Won't do a db solution that does concurrent writes, going to use a flat file because i value a simple setup.

I don't like the idea of waiting for the db, so maybe onDisable it saves the state to db. Or just is async but de-prioritised (as in the design of other classes doesn't rely on it's state), so is less wait critical.

Then i can replace the QuestDiary reading from the db with a QuestDiaryRegistry that does the same thing but as a class. Aka: db for preserving state, not for operations.

sammypanda commented 2 months ago

This issue needs testing as of 4b9ee32...

But as far as i can tell, the database is no longer relied on for the realtime data. But just used as a store to restore state from.

Currently, in restoring state, it still has concurrency issues. I imagine it would also happen for submitting state if two people happen to at the same time so this needs to be resolved.

[00:54:49] [Server thread/INFO]: [PlayerQuests]: Finished loading database quests into registry: [1, 2, beans-tester-bonus_15eea801-3dc5-4339-93cf-79f15320067c, 3, 4, 5, 6, beans-tester-bonus, 7]
[00:54:49] [ForkJoinPool.commonPool-worker-57/WARN]: Could not set or update quest progress for the 4 quest: stmt pointer is closed
[00:54:49] [ForkJoinPool.commonPool-worker-54/WARN]: Could not set or update quest progress for the 2 quest: stmt pointer is closed
[00:54:49] [ForkJoinPool.commonPool-worker-55/WARN]: Could not set or update quest progress for the beans-tester-bonus quest: stmt pointer is closed
[00:54:49] [ForkJoinPool.commonPool-worker-58/WARN]: Could not set or update quest progress for the 1 quest: The database has been closed
^ shows concurrency issue when submitting quest in db. You can see the worker threads 57,54,55,58.. but it doesn't need to run at the same time/be fast since it shouldn't be critical to realtime data.

My first thought is everything in Database.class should get put on one thread.

sammypanda commented 2 months ago

Upon testing today with 2 players: there seems to be shared state problems in the GUI again now. I noticed when someone using SelectBlock selects something, it may select the block for another player who is currently using SelectBlock.

sammypanda commented 2 months ago

fixed in 6d867978ff97d79a86cc82a21e205cf831b61a37 as far as i can tell.