Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Environment variables for paths used #337

Closed
Entrivax opened this issue Dec 29, 2023 · 10 comments
Closed

Environment variables for paths used #337

Entrivax opened this issue Dec 29, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@Entrivax
Copy link

I've seen the paths to the work directories like /tmp or /vods are hardcoded. I would love for them to be changed with environment variables. In my case, I have a huge performance hit when the download of a big VOD is done and need to be moved, because it's moving files between two volumes, it copies the file when moving instead of just a move operation.

I would like to put both the temp working directory and the vods directory in a same parent directory that I would then mount, instead of mounting the /tmp and /vods separately. (I mounted the /tmp because I'm using Docker with WSL, and it was increasing the WSL virtual drive by a huge amount.)

In my case, I would like to do something like this:

  • GANYMEDE_TEMP_DIR=/data/.tmp
  • GANYMEDE_VODS_DIR=/data/vods

and then mount the /data path.

@russelg
Copy link

russelg commented Jan 4, 2024

Seconding this, this is something I also need.

@Zibbp Zibbp added the enhancement New feature or request label Jan 4, 2024
@ccKep
Copy link

ccKep commented Jan 17, 2024

You should be able to already do this using the docker-compose.yml or am I wrong? Just change the mounts like this:

    volumes:
      - /data/vods:/vods
      - ./logs:/logs
      # notice this is ./data while the others are /data
      - ./data:/data
      - /data/.tmp:/tmp

@russelg
Copy link

russelg commented Jan 17, 2024

I can't find a particular official source on why things are this way, but root level mounts like /vods and /tmp are problematic due to how docker handles volumes. Ideally you would mount a single directory such as /data which would have both /data/vods and /data/tmp. This would allow instant moves as they won't be considered cross-filesystem.

From https://trash-guides.info/Downloaders/qBittorrent/Basic-Setup/

The default path setup suggested by some docker developers that encourages people to use mounts like /movies, /tv, /books or /downloads is very suboptimal and it makes them look like two or three file systems, even if they aren’t (Because of how Docker’s volumes work). It is the easiest way to get started. While easy to use, it has a major drawback. Mainly losing the ability to hardlink or instant move, resulting in a slower and more I/O intensive copy + delete is used.

And some other reading I could find on the matter:
https://www.reddit.com/r/docker/comments/nj8111/moving_files_between_volumes_in_docker_is_slow/gz5vbrj/
https://wiki.servarr.com/docker-guide#consistent-and-well-planned-paths

@FibreTTP
Copy link

Unfortunately, it's a bit deeper than just allowing the paths to be customised. File and folder moves are currently done by opening the source file, creating a destination file and writing the source file into it:

func MoveFile(sourcePath, destPath string) error {
log.Debug().Msgf("moving file: %s to %s", sourcePath, destPath)
inputFile, err := os.Open(sourcePath)
if err != nil {
return fmt.Errorf("error opening file: %v", err)
}
outputFile, err := os.Create(destPath)
if err != nil {
inputFile.Close()
return fmt.Errorf("error creating file: %v", err)
}
defer outputFile.Close()
_, err = io.Copy(outputFile, inputFile)
inputFile.Close()
if err != nil {
return fmt.Errorf("writing to output file failed: %v", err)
}
// Copy was successful - delete source file
err = os.Remove(sourcePath)
if err != nil {
log.Info().Msgf("error deleting source file: %v", err)
}
return nil
}

A "move" operation is never invoked, it's all copy delete.

I can't code, but to help, the temporary directories files are explicitly coded to be in /tmp here (vod) and here (live).

Once the directories are able to be customised, the move function shown would need to be changed to use os.Rename (more info on stackoverflow), which should be atomic.

@Zibbp
Copy link
Owner

Zibbp commented Jun 26, 2024

I initially opted to not use os.Rename as I wasn't 100% sure if it was atomic across all filesystems Unix-like systems and NFS shouldn't have issues. I know of some people using rclone as the backend which may have issues. I'll have to do some testing but refactoring to use os.Rename is a good idea.

@FibreTTP
Copy link

FibreTTP commented Jun 27, 2024

You might want to keep the current move function as a fallback. It seems that since os.Rename is a direct call to the OS's actual Rename function, if the source/destination is on a different filesystem/device, it will simply fail (you would then invoke the current copy and delete behaviour).

Another idea is to call the mv command, which handles that logic... but that is probably a bad idea.

@Zibbp
Copy link
Owner

Zibbp commented Jul 28, 2024

This will be possible in v3. See #474 for more information and instructions to beta test if you want to.

@FibreTTP
Copy link

FibreTTP commented Aug 2, 2024

Confirming this works (with HLS and instant moves) in the v3 beta!

@Entrivax
Copy link
Author

Thank you, this works so well! 🙏
This saves easily 20+ mins per VOD of 100% CPU (WSL probably doesn't help with that), especially with bigger VODs and when another download and convert are running at the same time.

I've set my paths like this and it does the job very well:

services:
  ganymede-api:
    environment:
      - VIDEOS_DIR=/data/videos
      - TEMP_DIR=/data/videos/.temp
      - LOGS_DIR=/data/logs
      - CONFIG_DIR=/data/config
      # ...
    volumes:
      - ./vods-storage:/data/videos
      - ./logs:/data/logs
      - ./config:/data/config

Just a little nitpick, it could be nice if the paths in the placeholders of the VOD creation/edition panel could reflect the path set in the environment variable, or at least update them with the new defaults of the docker-compose.yml
image

Anyway, thank you for this performance update! 🙏

@Zibbp
Copy link
Owner

Zibbp commented Aug 12, 2024

Glad to hear it's working better! For now I've updated the paths to be the new default (/data/videos), maybe in the future that can be dynamically pulled in.

@Zibbp Zibbp closed this as completed Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants