Make sure on reset we release file descriptors for open files
When using client Prometheus::Client.reset!
method we are not releasing File descriptors for the mmap file and file lock. In this case (if using reset! in tests, to get a fresh instance of Prometheus client), quite quickly you are reaching too many open files
exception.
To reproduce this, you can use the following script:
#!/usr/bin/env ruby
$LOAD_PATH << File.expand_path('../../lib', __FILE__)
require 'prometheus/client'
def fd_count
puts "Number of open files:"
puts `lsof -a -p #{Process.pid} | wc -l`
puts "====================="
end
prometheus = Prometheus::Client.registry
http_requests = prometheus.counter(:http_requests, 'A counter of HTTP requests made')
http_requests.increment
fd_count
100.times do
Prometheus::Client.reset!
end
fd_count
Before the fix (problem):
Number of open files:
26
=====================
Number of open files:
126
=====================
After the fix results:
Number of open files:
26
=====================
Number of open files:
26
=====================
The fix is made in two parts, one in MMAP C implementation, when unmap
is triggered we also need to release File descriptor.
Second, we never release file lock (Prometheus::Client::Helper::FileLock.unlock(path)
). So every other reset
we create a new file instead of sometimes re-using the same one (https://gitlab.com/gitlab-org/prometheus-client-mmap/-/blob/master/lib/prometheus/client/helper/mmaped_file.rb#L42)
Also for fresh reset, we need to wipe metric files. This behavior can be changed, if we instead of appending binary, would recreate the file each time we open it. I didn't go with this approach for now, as several tests will need to be reworked and bigger input would be needed.