Fix zfs pkg Diff() function to stop creating new tmp snapshots on every request

[rusenask]

Currently agent is relying on this info to stop idle jupyter. However, since it's always "new", it will not auto shutdown them.

[itamarst]

I'm assuming the temporary snapshot is there in order to get a self-consistent diff.

Options:

1. Don't use snapshot, diff might not be self-consistent. This is... probably fine for merging use case? Or is it not?
2. Agent could check not for new snapshots, but for new snapshots whose diff is null?

[itamarst]

Karolis explains real motivation:

the reason why it's using very weird things is apparently for performance (although I am pretty sure you can write it in a way where it is as fast as it is now and probably faster but also readable)
it needs to be fast because we are potentially querying it every second or two and it might have ~200K files that are "changed" and not committed yet
that was the issue when fusemachines did some training on a dataset with a lot of images and pretty much everything went into a meltdown :smile:
both gateway and runners were wasted
so the idea was to create these temp snapshots and even "cache" results. So then, in order to know the last change, you check when was the last time this cached result was created
with a temp snapshot and just return that time
to know which files have changed and when
the thing that's broken now is this caching mechanism, it creates snapshot every time

[itamarst]

My guess for what's going on: when latest snapshot and cached diff snapshot are the same, the Diff gets suspicious and deletes the cached diff snapshot. Unfortunately in a quiescent filesystem this is exactly what you'd expect, so the cached diff snapshot never survives.

I think the solution is to switch to checking dirty status in a more robust way, using written@snapshot (see https://unix.stackexchange.com/a/531248).

[itamarst]

https://zfsonlinux.topicbox.com/groups/zfs-discuss/Tec98e5ae1ff444e0 has more discussion and the problems with this, e.g. atime.

[prisamuel]

There is an epic thread on slack between Karolis and Luke about this. https://dotscience.slack.com/archives/C7PHSRL82/p1571067972354200

[itamarst]

My plan is to write a whole bunch of tests for Diff that actually use zfs. Then try switching to different implementations of dirty detection.

[itamarst]

Based on my interpretation of what GetDirtyDelta does, I've decided to use written@<snapshot> for dirty, and referenced for total. Here's a comparison of old and new code:

new dirty 0 == old dirty 0? new total 24576 == old total 24576?
new dirty 13824 == old dirty 13824? new total 26112 == old total 38400?
new dirty 14336 == old dirty 15360? new total 24576 == old total 39424?
new dirty 12800 == old dirty 12800? new total 25088 == old total 37888?

So similar but not identical.

[rusenask]

Seems that this is still an issue

[itamarst]

:cry: Yep:

root@05fdfa9c05e3:/# zfs list -r -t snapshot -o creation pool/dmfs/8a76851e-8f77-414d-a473-034fa86673dd@dotmesh-fastdiff
CREATION
Thu Mar 12 17:39 2020
root@05fdfa9c05e3:/# zfs list -r -t snapshot -o creation pool/dmfs/8a76851e-8f77-414d-a473-034fa86673dd@dotmesh-fastdiff
CREATION
Thu Mar 12 17:40 2020
root@05fdfa9c05e3:/# zfs list -r -t snapshot -o creation pool/dmfs/8a76851e-8f77-414d-a473-034fa86673dd@dotmesh-fastdiff

[itamarst]

Current theory: Something (presumably Jupyter) is writing data, so it's legit dirty? Except the plugin doesn't notice that, so maybe not? Will investigate further.

[itamarst]

New theory: it's the agent logs. They get stored on disk, in the dotmesh filesystem, and they keep being generated.

[itamarst]

The theory appears correct. The fix is now in agent master.