Skip to content

Respect context in file cache Get() and Head()#251

Merged
glin merged 1 commit intomainfrom
fix-cache
Jan 6, 2026
Merged

Respect context in file cache Get() and Head()#251
glin merged 1 commit intomainfrom
fix-cache

Conversation

@glin
Copy link
Contributor

@glin glin commented Dec 30, 2025

For https://github.com/rstudio/package-manager/issues/16862

Fix an issue with the file cache's Get() and Head() not respecting context, e.g. not properly timing out given a context with timeout.

@glin
Copy link
Contributor Author

glin commented Dec 30, 2025

Note: there's still an issue with file cache Get() can hang forever due to a race condition, but that is looking trickier to fix so I may do that separately.

Another issue is why the Cache.Get() calls hang in the first place when the first node should've retrieved the cache result already. Possibly a race condition where the cache job finishes after AddressedPush(), but before PollAddress(), so the node never gets notified of the cache job completing, and just hangs:

// Push a job into the queue. AddressedPush is a no-op if the queue
// already contains an item with the same address.
err = o.queue.AddressedPush(ctx, resolver.Priority, resolver.GroupId, address, resolver.Work)
if o.duplicateMatcher.IsDuplicate(err) {
// Do nothing since; someone else has already inserted the work we need.
slog.Debug(fmt.Sprintf("FileCache: duplicate address push for '%s'", address))
} else if err != nil {
value = &CacheReturn{
Err: err,
CacheKey: address,
}
return
}
// Find out when the job in the queue is done
errCh := o.queue.PollAddress(ctx, address)

Copy link
Contributor

@CDRayn CDRayn left a comment

Choose a reason for hiding this comment

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

Looks good to me. Approved.

@glin glin merged commit cc77e97 into main Jan 6, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments