DB size

[riddle]

Last time I checked (week ago?) DB’s size was ~ 135 MB. Today it’s 176 MB. I’m a bit worried about this trend. I’m not saying I don’t want big DB but it seems to get fatter and fatter with every day, like nothing was purged, ever.

What’s up with that?

[eliadil]

I think we agreed to delete records by hand, sometimes. I checked the file you uploaded - it has ~300k alive villagers and only around 4k dead villagers. Is it possible that there are 300k villagers alive on the server? Or does it signify that some records in db are created, but villager doesn't really spawn?

[riddle]

Infinite spawners maybe? They still try to spawn villagers, the entities are just not created.

[eliadil]

I'll have to test that. The event listener for creature spawn is set to ignoreCancelled events, but it wont hurt to double check.

[matejdro]

When you are checking, also check if it is MONITOR priority, otherwise something might cancel event after we check it.

[eliadil]

Infinite spawners are not a problem; the listener has MONITOR priority and ignores cancelled.

I think the problem is with regenerating villages - correct me if I'm wrong - when village regenerate, the villagers are deleted, and fresh batch of villies is spawned, right?

[riddle]

Yes.

5 villages regenerate every 15m, so every single one out of ~600 unclaimed will get swapped in a day (25h). There should be around 15-20 villies in a village.

The problem might be with kill events not firing on those villies then?

[matejdro]

I already recommended overriding villager tick and checking for .dead boolean. Bukkit events only trigger when entity is killed, where regeneration simply removes them (so they don't drop loot or perform any other dead-related actions).

[eliadil]

Mhm, I didn't really do anything with NMS classes (and I assume that is where its at), any hints how to get started and how is the class called?

[matejdro]

I think adding that to CustomVillager.java should do the trick (sorry I can't test it right now)

/**
     Entity update tick
*/
@Override
protected void bo() {
    super.bo();
    if (this.dead)
    {
        //Delete it here. It is probably also good idea to save deleted flag somewhere so you don't try to delete it multiple times (not sure how many times is this being called after entity dies)
    }
}

[eliadil]

Hm, the method is ok (it is the entity tick thingy) but it never goes into the if oO

[matejdro]

So even if you manually kill villager? I checked vanilla source a bit and it should go into if a least once.

[riddle]

BTW, can this solution also include some kind of code / command to purge villagers which don’t exist anymore? Or are we left with that big DB until the end of this map?

[matejdro]

I don't think so. Only way I see this being possible is stopping server and manually running script that would purge those since all chunks need to load.

[eliadil]

I killed villagers using lava, sword, and reloading villages with /mantle respawn - it didnt go into the if :[

Can it be connected to replacing villagers with custom villagers ?

[matejdro]

Yeah it definitely can be connected, but I think universal solution would be much better. I will check vanilla source tomorrow and see why is not working.

[riddle]

OK guys, this can be postponed. We just need to make sure that before new season starts, this is solved so we won’t end up with another 200-meg database.

[riddle]

Reopening because this DB size causes extreme slow down on server boot. I initially thought it was CommandHelper but no, after reducing it, removing TradeCraft sped the boot up. Then removing the DB from the plugin also caused the same result.

We need to take care of this. Even if it means maintenance and purging the DB from inexisting villies.

[riddle]

Oh and the DB is 270 MB now which means it got 70 MB more since three weeks ago! Anyone can work on this? I’m starting to think we might run out of disk space soon. It’s at 65% of SSD, because every plugins directory is ~170MB. Removing TradeCraft’s DB causes the size of backup to plummet to 54MB.

[riddle]

@eliadil @matejdro Can I get any update here, so I know what to expect? It’s worrying.

[eliadil]

I have no idea how to fix this reliably - from what I tested, the overrided methods (the one matejdro posted bo() or die()) never fired on our custom villager class. I also dont know how to remove the dead villagers from the db - I will test few ideas in the evening.

The only workaround I can think for now - mark the villager as to be saved to the db only when someone right click him for the first time. This would remove the need of saving villagers from freshly regenerated villages.

[riddle]

I also dont know how to remove the dead villagers from the db - I will test few ideas in the evening.

I’m open to rescanning the whole world locally (I can do it), chunk by chunk, saving IDs of alive villies and after all that is done, removing those all other IDs from the DB snapshot from production (skipping IDs generated after rescan timestamp to not delete any new villies used by players).

The only workaround I can think for now - mark the villager as to be saved to the db only when someone right click him for the first time.

This is a great idea!

[matejdro]

I'm working on solution for already dead villagers. So in worst case we can just run this periodically.

It needs server shut down though.

[matejdro]

Ok some crazy statistics:

on flatcore map from few months ago (you don't publish maps anymore), there are 16 336 villagers. In tradecraft database from latest plugins dump there are 654 646 villagers. How the hell did we get to that number?

[riddle]

Village regen. A village can hold up to 15 villies, 5 villages regen every 15 minutes: 300 per hour, 7200 per day, 216,000 per month.

[matejdro]

I'm starting to think we wasted all that time on optimizing database while this is so much more obvious solution.

[riddle]

No, it wasn’t a wasted time because my slowdowns during testing was with almost empty DB.

[matejdro]

OK nevermind then. Cleaner is almost ready.

[matejdro]

Here you go. https://github.com/CoreNetwork/VillagerCleaner

Similar to UUIDConverter, you need to put .jar into server file, shut down server and run it. Also don't forget to backup old db.

[riddle]

OK, any idea how long it will take? Is there any way for me to run it locally and copy the DB afterwards? I would announce to players that they would get X hours of villager rollback which is better than X hours of downtime.

[matejdro]

You can do it on any server with same world and db and then copy the db of course, but yeah it will be rollback.

I tested it on old map and it took around 30 minutes. Not that long, but not instant either. I recommend you do test run on your local server first to see how long it will take.

[riddle]

OK! Do you have an SSD btw?

[riddle]

BTW did you also load world_nether? There are villages there too.

[riddle]

Oh and sorry for triple posting, but do we have anything in current build of TradeCraft which would prevent this problem from reoccuring? Last commit was a month ago…

[matejdro]

Yes I have SSD, but server was on HDD.

Yes I load all 3 worlds (I know end has no villages, but just in case. It doesn't hurt).

Not yet, I will take a look at it later today.

[matejdro]

@eliadil This seems to work

    /**
     * Entity tick event
     */
    @Override
    public void h()
    {
        super.h();

        if (dead)
        {
            //Villager has died, delete it here
        }
    }

Will you do actual deletion since you know new DB system more than me?

[eliadil]

Sure.

[eliadil]

It catches the villagers being killed normally (wit a sword, for example) but not the /mantle respawn or just /butcher -n villie kills :[

[riddle]

Can’t Mantle just be modified to kill Villagers in a way that passes info to Mantle TradeCraft specifically?

[matejdro]

Yes it can be, but this is not very universal solution. Any other plugin or anything else could delete them without TC noticing.

[riddle]

Indeed. It’s puzzling to me how a plugin deleting mobs can’t fire any event that other plugins can pick up. But then I realized just now it’s why MobManager has those rescan loops if/when other plugins despawn mobs programatically, so that MM can update its limits and caps accordingly.

[riddle]

OK just updating that DB slimfast worked and it’s now at 8,4 MB down from 286 MB :D

I’d love to apply the patch today, maybe the Mantle idea is the best one for now?

[eliadil]

I think calling the tradecraft methods from mantle will work good for now, until we manage to find the better solution. The amount of villagers removed with other plugins is negligible.

Can you add it into mantle, @matejdro ? All that needs to be done on tradecraft side -

Villagers.getVillager(villager.getUniqueId().toString()).setDead(true);

[matejdro]

What if you override and I call die() method on CustomVillager? That way we don't have to add dependency to TradeCraft.

[riddle]

Where do we stand guys? I’d like to remove the issue once and for all, even if it’s a custom solution just for us, for now.

Ideas from the thread:

1. Only save to DB when a villager is right-clicked or
2. Add a Mantle call to TradeCraft asking it to delete villages removed during a village regen

I like the first idea way better because infinite spawners won’t be saving villagers if they weren’t interacted with in the first place.

[eliadil]

1st solution requires more work. Infinite spawners are not a problem, imo. With villagers-per-view-distance limits + good cleaning on normal death.

I'll test the thing matejdro mentioned, and if this doesnt work I'll add soft dependency + tradecraft call into mantle this evening.

Btw, mantle branch idiotfix doesnt require uuid conversions, right?

[riddle]

Btw, mantle branch idiotfix doesnt require uuid conversions, right?

Yes

[matejdro]

I can add to mantle if you want, you just add TC side.

[riddle]

Whatever is fastest to implement, guys. Right after this there will be UUID update and then new season development. Cannot spend two more weeks thinking about this issue, sorry.

[matejdro]

I can implement my side in few minutes. His side will probably take a while because he also needs to implement whole deleting thing.

[eliadil]

Wha.. No. Look few posts up. To mark villie for deletion all you have to call is :

Villagers.getVillager(uuid.toString()).setDead(true);

it will delete villie with next saving.