Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

update_music skips over new files with old timestamps #22

Closed
derat opened this issue Oct 5, 2021 · 3 comments
Closed

update_music skips over new files with old timestamps #22

derat opened this issue Oct 5, 2021 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@derat
Copy link
Owner

derat commented Oct 5, 2021

I think that there are cases where update_music can skip over music files that it doesn't know about yet due to them having mtimes and ctimes that precede the timestamp in last_update_time.json.

This is the relevant code from scanForUpdatedSongs in cmd/update_music/scan.go:

            stat := fi.Sys().(*syscall.Stat_t)
            ctime := time.Unix(int64(stat.Ctim.Sec), int64(stat.Ctim.Nsec))
            if fi.ModTime().Before(lastUpdateTime) && ctime.Before(lastUpdateTime) {
                return nil
            }

The problem is easy to trigger:

  1. Download two albums by the same artist.
  2. Move only one album under the music dir.
  3. Run update_music.
  4. Move the other album under the music dir.
  5. Run update_music again.

The songs in the second album won't be picked up by the second update_music run:

  • last_update_time.json will contain t3.
  • The artist dir's mtime and ctime will be t4.
  • The second album's mtime will be t1 and its ctime will be t4.
  • The songs in the second album will have an mtime and ctime of t1.

I should read more about how this is usually handled. Should scanForUpdatedSongs also process all the files in a directory if the directory's ctime is after the last update time? Do I need to walk multiple levels up the tree to handle the case where an entire artist directory gets moved (rather than just an album directory)?

@derat derat added the bug Something isn't working label Oct 5, 2021
@derat derat self-assigned this Oct 5, 2021
derat added a commit that referenced this issue Oct 5, 2021
Update check_music to support checking that all music files
under the music dir have been imported. This can be used to
detect the problem described in #22.

Also avoid checking for unused cover files if the song dump
doesn't contain cover filenames.
@derat
Copy link
Owner Author

derat commented Oct 5, 2021

Just to mention it, I also sometimes end up with unimported files when I merge duplicate songs and then forget to delete the corresponding files.

@derat
Copy link
Owner Author

derat commented Nov 16, 2021

I think that I can prevent this by tracking the directories that contained songs during the last update and then additionally processing songs in newly-seen directories.

I believe that I don't need to track each directory's mtime, since moving a file into an existing directory should update the file's ctime (which will make us process it).

@derat
Copy link
Owner Author

derat commented Nov 16, 2021

Interestingly, it seems to be up to the implementation whether the rename syscall updates a file's ctime or not. See https://unix.stackexchange.com/questions/211123/why-does-renaming-a-file-with-mv-command-alter-an-inodes-change-date-time and also https://pubs.opengroup.org/onlinepubs/009695399/functions/rename.html:

Some implementations mark for update the st_ctime field of renamed files and some do not. Applications which make use of the st_ctime field may behave differently with respect to renamed files unless they are designed to allow for either behavior.

It looks like Linux (ext4?) updates it, fortunately.

I also noticed that the ctime may not get updated (from Go's perspective, at least) if the rename happens very quickly after the file is created. I only saw this in unit tests, though, and I suspect it won't be a concern in real-world usage.

@derat derat closed this as completed in b80b017 Nov 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant