Skip to content

Fix mdadm collector for resync=PENDING. #309

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

Merged
merged 2 commits into from
Sep 19, 2016
Merged

Fix mdadm collector for resync=PENDING. #309

merged 2 commits into from
Sep 19, 2016

Conversation

SuperQ
Copy link
Member

@SuperQ SuperQ commented Sep 18, 2016

Add fix for mdadm devices in state resync=PENDING.

  • Update test and fixture.

@SuperQ SuperQ force-pushed the superq/mdstat branch 3 times, most recently from 300265c to 7d515fa Compare September 18, 2016 06:25
Add fix for mdadm devices in state `resync=PENDING`.
* Update test and fixture.
@@ -190,7 +190,7 @@ func parseMdstat(mdStatusFilePath string) ([]mdStatus, error) {

// If device is syncing at the moment, get the number of currently synced bytes,
// otherwise that number equals the size of the device.
if strings.Contains(lines[j], "recovery") || strings.Contains(lines[j], "resync") && !strings.Contains(lines[j], "resync=DELAYED") {
if strings.Contains(lines[j], "recovery") || strings.Contains(lines[j], "resync") && !strings.Contains(lines[j], "resync=DELAYED") && !strings.Contains(lines[j], "resync=PENDING") {
Copy link
Member

Choose a reason for hiding this comment

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

Can this take more values than DELAYED or PENDING? Should we just check if the line starts with resync=?

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked over the linux source0 and found that these are the only 2 cases. There is the other style of line that sometimes says resync =. Maybe we can make the parser smarter based on the source. It also seems like it always uses a \t in the code to print out that line. I can adjust the Contains

if strings.Contains(lines[j], "recovery") || strings.Contains(lines[j], "resync") && !strings.Contains(lines[j], "resync=DELAYED") {
if strings.Contains(lines[j], "recovery") ||
strings.Contains(lines[j], "resync") &&
!strings.Contains(lines[j], "\u0009resync=") {
Copy link
Member

Choose a reason for hiding this comment

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

\u0009 -> \t will be more understandable for people.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure if that was valid syntax, thanks, will fix.

Update `Contains` matching for `resync=`
@juliusv
Copy link
Member

juliusv commented Sep 19, 2016

👍 thanks

@juliusv juliusv merged commit f5a15ee into master Sep 19, 2016
@juliusv juliusv deleted the superq/mdstat branch September 19, 2016 15:32
@SuperQ SuperQ mentioned this pull request Nov 6, 2016
tamcore pushed a commit to gitgrave/node_exporter that referenced this pull request Oct 22, 2024
Add more needed variants of cpuinfo for mips64 and mips64le.

Signed-off-by: Ben Kochie <[email protected]>
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