fix(ermitegit): keep a refresh after a write from counting as a read of the repository
- commit
- 23f3a22b28b0346d9d05ad69fc553fd487f216f6
- author
- brice · · 2026-10-11 02:08 UTC
- parent
- 9de0f7d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +21 -4
M internal/adapters/ermitegit/cache.go +5 -4
| @@ -80,9 +80,10 @@ type handle struct { | |||
| 80 | 80 | c *cache | |
| 81 | 81 | e *entry | |
| 82 | 82 | repo *git.Repo | |
| 83 | + | peek bool // from peek: its release is not a use | |
| 83 | 84 | } | |
| 84 | 85 | ||
| 85 | - | func (h *handle) release() { h.c.release(h.e) } | |
| 86 | + | func (h *handle) release() { h.c.release(h.e, !h.peek) } | |
| 86 | 87 | ||
| 87 | 88 | // cache is a bounded LRU of open repositories keyed by directory. A repository | |
| 88 | 89 | // leaves it by eviction, by idling longer than the TTL, by Forget (the | |
| @@ -334,7 +335,7 @@ func (c *cache) peek(dir string) *handle { | |||
| 334 | 335 | return nil | |
| 335 | 336 | } | |
| 336 | 337 | e.users++ | |
| 337 | - | return &handle{c: c, e: e, repo: e.opened.Load().repo} | |
| 338 | + | return &handle{c: c, e: e, repo: e.opened.Load().repo, peek: true} | |
| 338 | 339 | } | |
| 339 | 340 | ||
| 340 | 341 | // refreshNow runs Refresh on the open repository of dir, if the cache has it, | |
| @@ -356,14 +357,14 @@ func (c *cache) refreshNow(ctx context.Context, name domain.RepoName, dir string | |||
| 356 | 357 | return nil | |
| 357 | 358 | } | |
| 358 | 359 | ||
| 359 | - | func (c *cache) release(e *entry) { | |
| 360 | + | func (c *cache) release(e *entry, use bool) { | |
| 360 | 361 | c.mu.Lock() | |
| 361 | 362 | e.users-- | |
| 362 | 363 | if e.dropped { | |
| 363 | 364 | if e.users == 0 { | |
| 364 | 365 | c.doomed = append(c.doomed, e) | |
| 365 | 366 | } | |
| 366 | - | } else { | |
| 367 | + | } else if use { | |
| 367 | 368 | e.lastUsed = time.Now() // a long read is a use, up to its end | |
| 368 | 369 | c.lru.MoveToFront(e.elem) | |
| 369 | 370 | } | |
| @@ -80,9 +80,10 @@ type handle struct { | |||||
| 80 | c *cache | 80 | c *cache | ||
| 81 | e *entry | 81 | e *entry | ||
| 82 | repo *git.Repo | 82 | repo *git.Repo | ||
| 83 | + | peek bool // from peek: its release is not a use | |||
| 83 | } | 84 | } | ||
| 84 | 85 | ||||
| 85 | - | func (h *handle) release() { h.c.release(h.e) } | 86 | + | func (h *handle) release() { h.c.release(h.e, !h.peek) } |
| 86 | 87 | ||||
| 87 | // cache is a bounded LRU of open repositories keyed by directory. A repository | 88 | // cache is a bounded LRU of open repositories keyed by directory. A repository | ||
| 88 | // leaves it by eviction, by idling longer than the TTL, by Forget (the | 89 | // leaves it by eviction, by idling longer than the TTL, by Forget (the | ||
| @@ -334,7 +335,7 @@ func (c *cache) peek(dir string) *handle { | |||||
| 334 | return nil | 335 | return nil | ||
| 335 | } | 336 | } | ||
| 336 | e.users++ | 337 | e.users++ | ||
| 337 | - | return &handle{c: c, e: e, repo: e.opened.Load().repo} | 338 | + | return &handle{c: c, e: e, repo: e.opened.Load().repo, peek: true} |
| 338 | } | 339 | } | ||
| 339 | 340 | ||||
| 340 | // refreshNow runs Refresh on the open repository of dir, if the cache has it, | 341 | // refreshNow runs Refresh on the open repository of dir, if the cache has it, | ||
| @@ -356,14 +357,14 @@ func (c *cache) refreshNow(ctx context.Context, name domain.RepoName, dir string | |||||
| 356 | return nil | 357 | return nil | ||
| 357 | } | 358 | } | ||
| 358 | 359 | ||||
| 359 | - | func (c *cache) release(e *entry) { | 360 | + | func (c *cache) release(e *entry, use bool) { |
| 360 | c.mu.Lock() | 361 | c.mu.Lock() | ||
| 361 | e.users-- | 362 | e.users-- | ||
| 362 | if e.dropped { | 363 | if e.dropped { | ||
| 363 | if e.users == 0 { | 364 | if e.users == 0 { | ||
| 364 | c.doomed = append(c.doomed, e) | 365 | c.doomed = append(c.doomed, e) | ||
| 365 | } | 366 | } | ||
| 366 | - | } else { | 367 | + | } else if use { |
| 367 | e.lastUsed = time.Now() // a long read is a use, up to its end | 368 | e.lastUsed = time.Now() // a long read is a use, up to its end | ||
| 368 | c.lru.MoveToFront(e.elem) | 369 | c.lru.MoveToFront(e.elem) | ||
| 369 | } | 370 | } | ||
M internal/adapters/ermitegit/cache_test.go +16 -0
| @@ -224,6 +224,22 @@ func TestIdleRepositoriesAreClosed(t *testing.T) { | |||
| 224 | 224 | assert.Equal(t, 1, a.OpenRepos()) | |
| 225 | 225 | } | |
| 226 | 226 | ||
| 227 | + | // A Refresh after a write is not a read: a repository pushed to but never read | |
| 228 | + | // still closes after IdleTTL. | |
| 229 | + | func TestRefreshIsNotAUse(t *testing.T) { | |
| 230 | + | ctx := context.Background() | |
| 231 | + | a, _, data := newAdapter(t, ermitegit.AllRoutes(ermitegit.ErmiteGit), func(c *ermitegit.Config) { c.IdleTTL = 80 * time.Millisecond }) | |
| 232 | + | f := commitOf(t, data, "pushed") | |
| 233 | + | _, err := a.ListRefs(ctx, f.Name) | |
| 234 | + | require.NoError(t, err) | |
| 235 | + | start := time.Now() | |
| 236 | + | for time.Since(start) < 5*time.Second && a.OpenRepos() == 1 { | |
| 237 | + | a.Refresh(ctx, f.Name) | |
| 238 | + | time.Sleep(20 * time.Millisecond) | |
| 239 | + | } | |
| 240 | + | assert.Equal(t, 0, a.OpenRepos(), "refreshes kept an unread repository open") | |
| 241 | + | } | |
| 242 | + | ||
| 227 | 243 | // The idle close does not cut a blob stream the reader still holds: the library | |
| 228 | 244 | // keeps the stream valid after Close. | |
| 229 | 245 | func TestIdleCloseDoesNotCutAStream(t *testing.T) { | |
| @@ -224,6 +224,22 @@ func TestIdleRepositoriesAreClosed(t *testing.T) { | |||||
| 224 | assert.Equal(t, 1, a.OpenRepos()) | 224 | assert.Equal(t, 1, a.OpenRepos()) | ||
| 225 | } | 225 | } | ||
| 226 | 226 | ||||
| 227 | + | // A Refresh after a write is not a read: a repository pushed to but never read | |||
| 228 | + | // still closes after IdleTTL. | |||
| 229 | + | func TestRefreshIsNotAUse(t *testing.T) { | |||
| 230 | + | ctx := context.Background() | |||
| 231 | + | a, _, data := newAdapter(t, ermitegit.AllRoutes(ermitegit.ErmiteGit), func(c *ermitegit.Config) { c.IdleTTL = 80 * time.Millisecond }) | |||
| 232 | + | f := commitOf(t, data, "pushed") | |||
| 233 | + | _, err := a.ListRefs(ctx, f.Name) | |||
| 234 | + | require.NoError(t, err) | |||
| 235 | + | start := time.Now() | |||
| 236 | + | for time.Since(start) < 5*time.Second && a.OpenRepos() == 1 { | |||
| 237 | + | a.Refresh(ctx, f.Name) | |||
| 238 | + | time.Sleep(20 * time.Millisecond) | |||
| 239 | + | } | |||
| 240 | + | assert.Equal(t, 0, a.OpenRepos(), "refreshes kept an unread repository open") | |||
| 241 | + | } | |||
| 242 | + | ||||
| 227 | // The idle close does not cut a blob stream the reader still holds: the library | 243 | // The idle close does not cut a blob stream the reader still holds: the library | ||
| 228 | // keeps the stream valid after Close. | 244 | // keeps the stream valid after Close. | ||
| 229 | func TestIdleCloseDoesNotCutAStream(t *testing.T) { | 245 | func TestIdleCloseDoesNotCutAStream(t *testing.T) { | ||