btrfs collector does not close file handles in a timely manner, which prevents unmounting

[hhoffstaette]

Host operating system: output of uname -a

Gentoo with kernel 6.4.10:
Linux tux 6.4.10 #1 SMP Wed Aug 9 16:27:28 CEST 2023 x86_64 Intel(R) Core(TM) i5-2400S CPU @ 2.50GHz GenuineIntel GNU/Linux

node_exporter version: output of node_exporter --version

node_exporter, version 1.6.0 (branch: non-git, revision: ff7f9d6)
build user: portage@localhost
build date: 20230807-19:11:32
go version: go1.20.7
platform: linux/amd64
tags: netgo osusergo static_build

node_exporter command line flags

btrfs collector is enabled :)

node_exporter log output

none

Are you running node_exporter in Docker?

no

What did you do that produced an error?

$mount /mnt/archive 
-> the mount worked and we wait one or two polling intervals

$umount /mnt/archive
umount: /mnt/archive: target is busy.
-> :(

$lsof | grep /mnt/archive
node_expo 20168                     node_exporter   12r      DIR               0,40       122        256 /mnt/archive
node_expo 20168 20171 node_expo     node_exporter   12r      DIR               0,40       122        256 /mnt/archive
node_expo 20168 20172 node_expo     node_exporter   12r      DIR               0,40       122        256 /mnt/archive
node_expo 20168 20190 node_expo     node_exporter   12r      DIR               0,40       122        256 /mnt/archive

$pwd
/root
-> $pwd is not on the mount point, so unmount should have worked
-> restart node_exporter:

$/etc/init.d/node_exporter restart
 * Stopping node_exporter ...                                                 [ ok ]
 * Starting node_exporter ...                                                 [ ok ]

$umount /mnt/archive
-> success!

What did you expect to see?

Working unmount without having to restart node_exporter

What did you see instead?

Unmount not working

Additional observation

This behaviour is 100% repeatable and does not resolve itself over time.

Edit: apparently it does resolve itself over time, as long as the GC is triggered.

[dswarbrick]

Do the node_exporter file descriptors accumulate with each scrape, or remain constant after the initial scrape?

I am so far not able to reproduce the issue. I can freely mount and unmount btrfs filesystems whilst node_exporter is running and being actively scraped. lsof sometimes shows that several PIDs of node_exporter have open FDs for the mountpoint, but this seemingly disappears if the FS is unmounted and remounted whilst node_exporter is running. Restarting node_exporter also does not show FDs for the mountpoint again.

[hhoffstaette]

They remain constant after each scrape. I also just found that curl-ing node_exporter a few times does cause the open fds to disappear after the inital scrape, presumably due to GC and fds being collected (?). However that is still not great - external resources should never be coupled to GC lifetimes. Also, once they have disappeared, they reappear on the next scrape.

this seemingly disappears if the FS is unmounted

But that is the whole problem: you cannot unmount a filesystem while it has open fds.

[dswarbrick]

It seems pretty hit and miss that I'm actually able to get node_exporter to show open FDs for the btrfs mountpoint, however on the rare occasion that lsof shows open FDs, they of course prevent unmounting of the filesystem. If the node_exporter is scraped a second time, the open FDs disappear, and the filesystem can naturally be unmounted at that point.

This is obviously a pretty artificial scenario, where the node_exporter only gets scraped exactly one time in its whole uptime. But the behaviour also smells a bit racy to me.

[hhoffstaette]

I agree that the behaviour is mostly harmless for permanently mounted filesystems (e.g. in a server); in this case the /mnt/archive drive is an external device that is only mounted manually & on-demand, and which I need to unmount when I'm done. So this mostly affects removable devices.

[dswarbrick]

I totally agree that one should be able to mount and unmount filesystems independently of node_exporter. My point was more about the fact that (at least for me), these "stale" FDs immediately disappear with the second scrape. So unless you're extraordinarily unlucky and try to unmount a btrfs at just the right moment in time when a freshly started node_exporter has only completed one scrape, I'm not sure how often it would be an issue.

Nevertheless, this is flaky, and should be resolved.

[SuperQ]

Sounds like we may be missing a defer close() somewhere.

[dswarbrick]

@SuperQ Yeah, collector/btrfs_linux.go calls btrfs.Open() and never actually calls FS.Close() (which would close the underlying FDs in the btrfs.FS struct).

It's not entirely clear to me how the leaky FDs are actually closing after the second scrape (and quite difficult to provoke in the first place), since I see nothing in collector/btrfs that would ever close the btrfs FDs.

[hhoffstaette]

@SuperQ Yeah, collector/btrfs_linux.go calls btrfs.Open() and never actually calls FS.Close() (which would close the underlying FDs in the btrfs.FS struct).

It's not entirely clear to me how the leaky FDs are actually closing after the second scrape (and quite difficult to provoke in the first place), since I see nothing in collector/btrfs that would ever close the btrfs FDs.

File handles in Go are eventually GC-collected via finalizer, see here.

[hhoffstaette]

Fix works as expected - thanks! ❤️