Implementation of fsnotify based scan watcher

[brian-doherty]

Tested on Linux.

[sentriz]

thanks Brian!! this looks nice . hoping to have a closer look tomorrow

[sentriz]

might be nice to extract this as a "newScanContext" or something, IIRC we do the same thing above

[brian-doherty]

Not sure I follow since I'm doing the same with the local context creation as in ScanAndClean()

[sentriz]

what case is this handing? do we support watching normal files?

[brian-doherty]

If memory serves this is handling the sym link case.

[sentriz]

lookin good. and left some questions above.

i'm also wondering about limits that the kernal has on inotify watchers per user. apparently it's 8192 on ubuntu. my personal library is approaching 8k dirs. what would happen if someone with a bigger library starts the watcher?

also wondering how it affects memory consumption

cheers! sorry for the very late responses here btw

[sentriz]

also interestingly, the likes of jellyfin and plex do filewatching too. i wonder do they have some trick up their sleeve to avoid the limits

[brian-doherty]

Re: watch limits -- if someone with more directories starts the watcher they should see errors in the log but it will watch what it can. Then if they have root access they can increase the system limit.

[sentriz]

just ran again locally and seems to work quite nice 👍

i have some questions also

• when starting gonic for the first time, it seems to do a scan of the root dir straight away. it it meant to do that? could be annoying for users with big libraries. and it feels like it shouldn't since there wasn't any explicit event from the kernel
• are delete events supported? i did an rm * in my root dir and it didn't seem to pick it up. that's fine though i think. just wondering
• perhaps we should debounce the events. eg if i have a folder outside my library like

  artistname/
  artistname/albumname/
  artistname/albumname/track1.flac
  artistname/albumname/track2.flac
  

  and copy that into my gonic root dir like

  $ cp -r artistname gonicroot/
  

  it picks up the artistname event straight away but never finds the album or tracks, since we haven't copied at the time of the first artistname/ event. have to do a manual scan. (at least for me in my testing)
• also related to the deboucing, if we do

  for track in somefiles/*.mp3; do
      cp "$track" gonicroot/someartist/some/album/
  end
  

  (which simulates some slower copies - maybe over a remote filesystem or whatever) we doing a lot of scans. which is maybe fine - but if one scan takes longer than it does to copy to files, we miss the second event. i think a debounce helps us here too

what do you think? IIRC syncthing for example does some debounce stuff

[sentriz]

another QQ, could s.watchMap be replaced by s.watcher.WatchList() ?

also regarding the debounce, if you think it's a good idea, hugo do something similar here
https://github.com/gohugoio/hugo/blob/master/watcher/batcher.go

(found from fsnotify/fsnotify#401 )

[brian-doherty]

Re: initial scan -- yes, that was deliberate to make sure the library was up to date as we set the watches, but I'm fine with omitting it if that's what you want.
Re: deletion -- I explicitly did not handle deletion as I figured it would get handled in the periodic scan just fine.
Re: whole tree copy in -- that should work. I will have a look. Are you sure you didn't hit your max watch limit? Because in that case you'd see exactly the symp[tom you report.
Re: watchMap -- we need the mapping back to which music dir root each directory is in in order to pass to the scanCallback

[sentriz]

Re: initial scan -- yes, that was deliberate to make sure the library was up to date as we set the watches, but I'm fine with omitting it if that's what you want.

yeah personally i think it would be good to not scan on startup, if that doesn't break anything

Re: deletion -- I explicitly did not handle deletion as I figured it would get handled in the periodic scan just fine.

👍

Re: whole tree copy in -- that should work. I will have a look. Are you sure you didn't hit your max watch limit? Because in that case you'd see exactly the symp[tom you report.

thanks, yes this is just testing on my laptop with just a few dirs

Re: watchMap -- we need the mapping back to which music dir root each directory is in in order to pass to the scanCallback

👍

[sentriz]

ah now it has started working for the whole tree copy. though it produces a lot of event. i think if we debounce/batch (maybe something like the hugo example?) it help a lot. any maybe make sure we don't miss any files

for example: ("watching" message turned off)

20220913_202500.mp4

[brian-doherty]

We could go a couple of routes on the batching:

1. Batch them all and just do a full scan when there's a change. Probably pretty wasteful.
2. Batch and eliminate dup directories.
3. Not bother batching and maybe reduce the logging on this thing so that it's not so noticeable.

I prefer 3 because of sloth :D but I'd be willing to take a stab at 2.

[sentriz]

personally like 2

not only seems like a good thing but I think solves the problem I initially had when I first tested (whole tree copied, notified only about first event, no children to add watchers to yet, meaning no children found (though I could be way off on this))

wdyt?

very happy to help out on this if your time is limited

[brian-doherty]

Let me see if I can get to it this week.

[brian-doherty]

Ok check it out.

[sentriz]

thanks brian! did some testing and seem to work very well. i just make scanList a map (seems like we can just incase we get duplicate events, then we don't try process them twice. does this make sense?) and removed the "watching folder" log message, since it's a lot with a big library

is this ok?

[brian-doherty]

OK by me. Thanks!