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

Added config option to set size of transcode cache and cadence to enf… #520

Merged
merged 6 commits into from
Sep 15, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ password can then be changed from the web interface
| `GONIC_MULTI_VALUE_GENRE` | `-multi-value-genre` | **optional** setting for multi-valued genre tags when scanning ([see more](#multi-valued-tags-v016)) |
| `GONIC_MULTI_VALUE_ARTIST` | `-multi-value-artist` | **optional** setting for multi-valued artist tags when scanning ([see more](#multi-valued-tags-v016)) |
| `GONIC_MULTI_VALUE_ALBUM_ARTIST` | `-multi-value-album-artist` | **optional** setting for multi-valued album artist tags when scanning ([see more](#multi-valued-tags-v016)) |
| `GONIC_TRANSCODE_CACHE_SIZE` | `-transcode-cache-size` | **optional** size of the transcode cache in MB (0 = no limit) |
| `GONIC_TRANSCODE_EJECT_INTERVAL` | `-transcode-eject-interval` | **optional** interval (in minutes) to eject transcode cache (0 = never) |
| `GONIC_EXPVAR` | `-expvar` | **optional** enable the /debug/vars endpoint (exposes useful debugging attributes as well as database stats) |

## multi valued tags (v0.16+)
Expand Down
Binary file added cmd/gonic/gonic
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this was committed accidentally

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. Fixed.

Binary file not shown.
17 changes: 17 additions & 0 deletions cmd/gonic/gonic.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ func main() {

deprecatedConfGenreSplit := flag.String("genre-split", "", "(deprecated, see multi-value settings)")

confTranscodeCacheSize := flag.Int("transcode-cache-size", 0, "size of the transcode cache in MB (0 = no limit) (optional)")
confTranscodeEjectInterval := flag.Int("transcode-eject-interval", 0, "interval (in minutes) to eject transcode cache (0 = never) (optional)")

flag.Parse()
flagconf.ParseEnv()
flagconf.ParseConfig(*confConfigPath)
Expand Down Expand Up @@ -201,6 +204,7 @@ func main() {
transcoder := transcode.NewCachingTranscoder(
transcode.NewFFmpegTranscoder(),
cacheDirAudio,
*confTranscodeCacheSize,
)

lastfmClientKeySecretFunc := func() (string, string, error) {
Expand Down Expand Up @@ -404,6 +408,19 @@ func main() {
return nil
})

errgrp.Go(func() error {
if *confTranscodeEjectInterval == 0 || *confTranscodeCacheSize == 0 {
return nil
}

defer logJob("transcode cache eject")()

ctxTick(ctx, time.Duration(*confTranscodeEjectInterval)*time.Minute, func() {
transcoder.CacheEject()
})
return nil
})

errgrp.Go(func() error {
if *confScanIntervalMins == 0 {
return nil
Expand Down
4 changes: 4 additions & 0 deletions contrib/config
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ playlists-path <path to your m3u playlist dir>
# regenerated.
cache-path /var/cache/gonic

# Option to eject least recently used items from transcode cache.
#transcode-cache-size 5000 # in Mb (0 = no limit)
#transcode-eject-interval 1440 # in minutes (0 = never eject)

# Interval (in minutes) to check for new music. Default: don't scan
#scan-interval 0
#scan-at-start-enabled false
Expand Down
2 changes: 1 addition & 1 deletion transcode/transcode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestCachingParallelism(t *testing.T) {
callback: func() { realTranscodeCount.Add(1) },
}

cacheTranscoder := transcode.NewCachingTranscoder(transcoder, t.TempDir())
cacheTranscoder := transcode.NewCachingTranscoder(transcoder, t.TempDir(), 1024)

var wg sync.WaitGroup
for i := 0; i < 5; i++ {
Expand Down
43 changes: 41 additions & 2 deletions transcode/transcoder_caching.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,24 @@ import (
"io"
"os"
"path/filepath"
"sort"
"sync"
"time"
)

const perm = 0o644

type CachingTranscoder struct {
cachePath string
transcoder Transcoder
limitMb int
locks keyedMutex
}

var _ Transcoder = (*CachingTranscoder)(nil)

func NewCachingTranscoder(t Transcoder, cachePath string) *CachingTranscoder {
return &CachingTranscoder{transcoder: t, cachePath: cachePath}
func NewCachingTranscoder(t Transcoder, cachePath string, limitMb int) *CachingTranscoder {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks megabits to me. how about a big B for bytes

Suggested change
func NewCachingTranscoder(t Transcoder, cachePath string, limitMb int) *CachingTranscoder {
func NewCachingTranscoder(t Transcoder, cachePath string, limitMB int) *CachingTranscoder {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return &CachingTranscoder{transcoder: t, cachePath: cachePath, limitMb: limitMb}
}

func (t *CachingTranscoder) Transcode(ctx context.Context, profile Profile, in string, out io.Writer) error {
Expand Down Expand Up @@ -52,6 +55,7 @@ func (t *CachingTranscoder) Transcode(ctx context.Context, profile Profile, in s

if i, err := cf.Stat(); err == nil && i.Size() > 0 {
_, _ = io.Copy(out, cf)
_ = os.Chtimes(path, time.Now(), time.Now()) // Touch for LRU cache purposes
return nil
}

Expand All @@ -64,6 +68,41 @@ func (t *CachingTranscoder) Transcode(ctx context.Context, profile Profile, in s
return nil
}

func (t *CachingTranscoder) CacheEject() {
Copy link
Owner

@sentriz sentriz Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could return an error, which we can just log in main.go

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// Delete LRU cache files that exceed size limit. Use last modified time.
type file struct {
path string
info os.FileInfo
}

var files []file
var total int64 = 0

_ = filepath.Walk(t.cachePath, func(path string, info os.FileInfo, err error) error {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use the new (and faster i belive) filepath.WalkDir here?

and return an error too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if err != nil {
return err
}
if !info.IsDir() {
files = append(files, file{path, info})
total += info.Size()
}
return nil
})

sort.Slice(files, func(i, j int) bool {
return files[i].info.ModTime().Before(files[j].info.ModTime())
})

for total > int64(t.limitMb)*1024*1024 {
curFile := files[0]
files = files[1:]
total -= curFile.info.Size()
unlock := t.locks.Lock(curFile.path)
Copy link
Owner

@sentriz sentriz Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the t.locks is a keyedMutex but the keys are profile strings (ffmpeg commands with args and paths) not paths. so this has little effect i think

maybe we should have a global clean lock which the normal transcode thing respects too?

something like

type CachingTranscoder struct {
    ...
    locks      keyedMutex
    cleanLock  sync.RWMutex
}

...

func (t *CachingTranscoder) Transcode(...) error {
    ...
    
    t.cleanLock.RLock()
    defer t.cleanLock.RUnlock()
    
    ...
    
    key := cacheKey(name, args)
    unlock := t.locks.Lock(key)
    defer unlock()
}

func (t *CachingTranscoder) CacheEject() error {
    t.cleanLock.Lock()
    defer t.cleanLock.Unlock()

    ...
}

that way the cleaning process locks the entire cache, while multiple goroutines looking to read the cache work fine, just not while we're cleaning

wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call and my apologies. Done.

_ = os.Remove(curFile.path)
unlock()
}
}

func cacheKey(cmd string, args []string) string {
// the cache is invalid whenever transcode command (which includes the
// absolute filepath, bit rate args, replay gain args, etc.) changes
Expand Down
Loading