from a downstream conversation with <@U6YPB4SJX> a...
# developmenta
aloof-angle-91616
02/08/2019, 4:55 PMfrom a downstream conversation with @average-vr-56795 about lmdb gc with pantsd active (also noting that currently we don't do any gc without pantsd active, which i think i'm fine with)
dwagnerhall [00:53]
FWIW the LMDB garbage collection we do is also kind of crappy because it doesn’t necessarily compact… Not sure of a good strategy for doing some background garbage collection without blocking user action…
dmcclanahan [Today at 00:54]
? is blocking user action necessary for compacting?
9 replies
dwagnerhall [8 hours ago]
Garbage collecting at all requires an exclusive lock on the store, so pants will be unable to do anything
dwagnerhall [8 hours ago]
Compacting requires copying the entries we want to keep to a new directory, deleting the existing one, and renaming over the top, which then requires resetting the Store in any process which has it open, which basically is a pantsd restart if you’re using pantsd
dmcclanahan [8 hours ago]
can we shard with a partition key of some sort to avoid a global store lock? i do not know what renaming is referring to unless you just mean moving the file that keeps the data and then resetting to point to the new file
dwagnerhall [8 hours ago]
We already shard 16 ways, but fundamentally, garbage collecting 1/16 of the store is still going to stop pants from being able to do anything while that 1/16 is locked
dwagnerhall [8 hours ago]
And yes, the renaming is for the backing file that keeps the data
dwagnerhall [8 hours ago]
The lmdb format doesn’t allow for compacting when you delete, so the way we “compact” is by iterating over the entries to decide which ones we want to keep, then copying those to a new dir, and deleting the old one
dmcclanahan [8 hours ago]
i don't immediately have a solution for that
dmcclanahan [6 minutes ago]
also forgot that we shard exactly like that already i remember the ShardedLmdb code was so slick
dmcclanahan [1 minute ago]
ok so i’m confused as to how the method you describe requires blocking pants — are we ever deleting anything from the db except for garbage collection? can we do the copying without blocking and only block on the rename? and it seems intrinsically like there should be a way to reset the store without restarting pantsd, not that i know what i’m talking about...
aloof-angle-91616
02/08/2019, 4:57 PMand also @witty-crayon-22786 we could maybe at least send the user a message if pantsd isn't activated after the store reaches a certain size saying "hey, the lmdb store is getting pretty big! pantsd garbage collects the store, so you may want to consider turning that on!"
a
average-vr-56795
02/08/2019, 5:04 PMWe probably shouldn’t suggest people turn pantsd on given it’s know-buggy… But it would be trivial to add a a
garbage-collect-lmbd-store👌 1
average-vr-56795
02/08/2019, 5:04 PMThere are two things that necessitate some kind of synchronisation:
a
aloof-angle-91616
02/08/2019, 5:05 PM(i would be in favor of that and yes not super ideal until we can add a test harness for pantsd and be confident about it)
a
average-vr-56795
02/08/2019, 5:05 PM1. The process of working out what to delete requires an exclusive lock, because we need to work out what is and isn’t in use in the graph (and so what we can and can’t delete)
average-vr-56795
02/08/2019, 5:06 PM2. Actually doing the delete and rename requires the caller to re-create their
Storew
witty-crayon-22786
02/08/2019, 5:20 PMOne time cleanup when pantsd goes on by default would also be easy, and my vote vs compaction
witty-crayon-22786
02/08/2019, 5:21 PM(for the near term)
a
average-vr-56795
02/08/2019, 5:21 PMI agree with that, but also, there’s still the “garbage collection doesn’t compact” issue…
a
aloof-angle-91616
02/08/2019, 5:29 PM(1) makes sense, although i have suspicions that can likely be assuaged by looking at the code
aloof-angle-91616
02/08/2019, 5:30 PM(2) also makes sense in light of (1)
a
average-vr-56795
02/08/2019, 5:30 PM2 is unrelated to 1
average-vr-56795
02/08/2019, 5:30 PM2 is about stale fds
a
aloof-angle-91616
02/08/2019, 5:31 PMok
aloof-angle-91616
02/08/2019, 5:32 PMwhy does recreating the store imply an exclusive lock? does the graph hold cursors that are invalidated if the fd is retaken?
a
average-vr-56795
02/08/2019, 5:33 PMIt’s not quite an exclusive lock
average-vr-56795
02/08/2019, 5:33 PMOne pants process needs to be able to signal to all others that they need to reset their stores
a
aloof-angle-91616
02/08/2019, 5:33 PMok
a
average-vr-56795
02/08/2019, 5:33 PMAnd that kind of needs to be a “Notify I’m about to delete it”, “Notify you can recreate your Stores” kind of protocol
average-vr-56795
02/08/2019, 5:33 PMWhich is effectively a lock 🙂
a
aloof-angle-91616
02/08/2019, 5:34 PMok then we are on the same page as far as my understanding at this second goes