Skip to content

DirectFileStore : add hostname to identify a process #323

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

Closed
wants to merge 1 commit into from

Conversation

BenoitMC
Copy link

@BenoitMC BenoitMC commented May 5, 2025

Our application runs on a single physical host but in multiple Docker containers that share the same persistent volume (e.g., multiple Sidekiq or Puma + Sidekiq). Due to PID namespace isolation, the Ruby process PID inside each container is the same. As a result, the DirectFileStore class ends up using the same files across containers, which leads to file corruptions during concurrent writes.

This PR modifies the process identifier to include the hostname, ensuring that each container writes to a distinct file and preventing cross-container write conflicts.

@SuperQ
Copy link
Member

SuperQ commented May 5, 2025

This seems like the wrong solution to the problem.

Files should not be share across containers. Each container should have an internal metrics store.

@dmagliola
Copy link
Collaborator

dmagliola commented May 5, 2025

In your opinion, is this a standard way of running Ruby apps? It feels like an unusual setup to me, but I may be lacking exposure to real situations.

Unless this is very common, I feel like it would be better for you to subclass DirectFileStore, override this method, and then set your subclass as the store in the config. That would achieve the same outcome for you without changing the gem for all users.

I'd also suggest not overriding process_id, but instead overriding the method that generated the actual file path (filemap_filename)

@SuperQ
Copy link
Member

SuperQ commented May 5, 2025

You don't want volume mount the metrics file store. It is standard to use a tmpfs memory store inside each container.

Otherwise you can't keep the metrics lifecycle synced with the container lifecycle.

@BenoitMC
Copy link
Author

BenoitMC commented May 5, 2025

Thank you for your responses. I’m just getting started with Prometheus. The idea was to have X Sidekiq containers storing their metrics in the same folder (but in separate files), which would then be aggregated and scraped through a single target.

Should I instead separate them and have as many targets as there are containers?

@BenoitMC BenoitMC closed this May 5, 2025
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.

3 participants