Conversation
…the connection to avoid deadlocks.
|
Some of these are definitely my bad from lack of knowledge, more specifically the use of Thank you for the reviews and pull requests! |
|
Test my changes thoroughly before putting them live. I'm a dumdum just like everybody else. |
Aleksey-Terzi
left a comment
There was a problem hiding this comment.
Overall looks good.
A couple of things:
-
Not sure about Timer class, looks like we will be getting too much spam in the log?
-
Good work on batches, though maybe we don't need batches at all?
Previously we didn't have a periodic data saving and therefore the plugin was saving all changes accumulated for the day.
Though now we save data every 1 min (the async task in CivModCore) and the shutdown just saves data that wasn't saved in the last 1 min.
At the same time, batches make code more complicated.
Therefore I propose to delete the batching at all.
| continue; | ||
|
|
||
| // Add to a list to avoid connection deadlocks. | ||
| List<Reinforcement> inserts = new ArrayList<>(); |
There was a problem hiding this comment.
Not sure we need to create this list here.
The consumer function is pretty fast, this is what the consumer calls in the end (isNew = false):
https://github.com/CivMC/CivModCore/blob/master/paper/src/main/java/vg/civcraft/mc/civmodcore/world/locations/chunkmeta/block/BlockBasedChunkMeta.java#L227
Updated CitadelDAO with some preemptive fixes. This should add some clarity to deadlocking issues when diagnosed.
Adressed issues: