Skip to content

Try to patch inotify#4672

Open
iliasu-zinc wants to merge 2 commits intolima-vm:masterfrom
iliasu-zinc:chore/attempt-inotify-patch
Open

Try to patch inotify#4672
iliasu-zinc wants to merge 2 commits intolima-vm:masterfrom
iliasu-zinc:chore/attempt-inotify-patch

Conversation

@iliasu-zinc
Copy link
Copy Markdown

Description

When using virtiofs and inotify it seems that you can intermittently hit permission denied errors for example when running yarn install in a decently sized repository. These errors don't actually seem to be a problem with virtiofs.

virtiofs = works fine but no hot reload inside containers
virtiofs + mount inotify = hot reload inside container works but yarn install will crash with a permission denied issue

With this change hopefully we see the following;

virtiofs + mount inotify = yarn install works + hot reload works

This PR seemingly achieves this aim, but I haven't worked on Go in a long time so I'm not an expert this fix is only me debugging with strace and trying to patch what I see.

Testing Done

  1. yarn install inside a Docker container with a virtiofs-mounted volume completes without EACCES errors
  2. Editing a source file on the host triggers a single hot-reload cycle (no loops) <- this was a side-effect of an earlier revision of this change, I included it to try to show how the testing was conducted.

type agent struct {
// Ticker is like time.Ticker.
// We can't use inotify for /proc/net/tcp, so we need this ticker to
// reload /proc/net/tcp.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why remove?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Squash the commits
  • Add Signed-off-by line to the commit message to sign the DCO
  • Make the PR title descriptive, please

socketLister *sockets.Lister
kubernetesServiceWatcher *kubernetesservice.ServiceWatcher
runtimeDir string
recentChtimes map[string]time.Time
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs a comment line to explain the key

a.recentChtimes[location] = now

// Prevent unbounded growth during bulk operations
if len(a.recentChtimes) > 10000 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

10000 should be defined as a const

}
a.recentChtimes[location] = now

// Prevent unbounded growth during bulk operations
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use LRU algorithm or something?

@AkihiroSuda AkihiroSuda modified the milestone: v2.1.0 Mar 8, 2026
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