-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix mount points being collected multiple times in filesystem_linux #3376
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
Fix mount points being collected multiple times in filesystem_linux #3376
Conversation
c347b31 to
83efcfe
Compare
|
@SuperQ do you have an opinion on this? Do you think this minimal fix is a good approach or would you like a more obvious/clearer implementation here? |
|
@SuperQ @discordianfish Just pinging for an update :) Are you interested in this change? I'm pretty sure this issue still exists. @aequitas are you still seeing these errors in your log messages? Is it possible for you to test if this change fixes the issue for you? |
|
@0xf09f95b4 I'll try to test this change this week and let you know if it fixes things |
|
This needs a rebase after #3387. Yea, this is tricky to fix cleanly as we're overloading the "labels" with non-label filesystem information. |
Signed-off-by: Markus Sütter <[email protected]>
83efcfe to
de310e3
Compare
|
Thanks for your feedback! I just tested 1.10.0 and I can still see the issue happening. I rebased the commit. I had to modify the patch to now zero our both mount option strings. I also checked and as far as I can see, those options are still not used anywhere after the changed line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. This seems like a reasonable workaround for now. Fixing this cleanly would probably require some serious thought / refactoring of the whole filesystem collector flow.
* [BUGFIX] filesystem: Fix mount points being collected multiple times #3376 * [BUGFIX] filesystem: Refactor mountinfo parsing #3452 * [BUGFIX] meminfo: Add Zswap/Zswapped metrics #3453 Signed-off-by: Ben Kochie <[email protected]>
* [BUGFIX] filesystem: Fix mount points being collected multiple times #3376 * [BUGFIX] filesystem: Refactor mountinfo parsing #3452 * [BUGFIX] meminfo: Add Zswap/Zswapped metrics #3453 Signed-off-by: Ben Kochie <[email protected]>
Hi,
this PR attempts to address #2805.
The issue we encountered is a case in which a path is bind-mounted to itself, the same issue you can see in this latest comment.
NixOS bind-mounts the
/nix/storepath to/nix/storeto remount the same mount read-only and (nosuid, nodev), as you can see here. If the/nix/storepath is already a mount from some device, this will lead to metrics being collected twice for the same (mountPoint, device) combination if the mount options differ.This issue only occurs, when the filesystem options of the two mounts are different. The core issue is, that the existing filter that wants to prevent this issue here takes the entire
labelsstruct into account, including mount options.The mount options themselves are not used or exposed by node_exporter. So this minimal fix only removes the filesystem options after they are used to extract the
rogauge. This way, theseen := map[filesystemLabels]bool{}filter works correctly (as the options are always"").This has a side-effect: Previously, the
rogauge was exported twice in this case, if the two mounts were read only and read-write (oncero, oncerw). Now, the gauge is exported only once and in the example, it would be exportedrw, even though the mount actually used by the kernel (the upper mount), isro.Additional options we came up with to fix this:
optionsfromfileSystemLabelsalltogether (only used in Linux anyway).rostate.rooption to gauges as a label.@discordianfish You were involved in the original issue. What do you think?