Skip to content

Don't panic if readdrivestat returns negative value#2

Merged
lufia merged 2 commits intolufia:masterfrom
VladRassokhin:dont-panic
Sep 2, 2021
Merged

Don't panic if readdrivestat returns negative value#2
lufia merged 2 commits intolufia:masterfrom
VladRassokhin:dont-panic

Conversation

@VladRassokhin
Copy link
Contributor

@VladRassokhin VladRassokhin commented Sep 1, 2021

Context:
We observed several node_exporter panics on a machine which is used to create disk images.
Machine it could have more than 16 disks or add/remove them while readdrivestat is running, so some error in IOKit calls could arise.
That should not lead to a node_exporter failure though.

Here's an example panic log:

panic: runtime error: makeslice: len out of range

goroutine 188748 [running]:
github.com/lufia/iostat.ReadDriveStats(0x40741c0, 0xc0004c7d98, 0xf1b8b744, 0xc000081200, 0xc0004c7da8)
        github.com/lufia/iostat@v1.1.0/iostat_darwin.go:21 +0x86
github.com/prometheus/node_exporter/collector.(*diskstatsCollector).Update(0xc00028c330, 0xc000330b40, 0x4b04200, 0xc00023e1c0)
        github.com/prometheus/node_exporter/collector/diskstats_darwin.go:190 +0x34
github.com/prometheus/node_exporter/collector.execute(0x46d28ac, 0x9, 0x478d080, 0xc00028c330, 0xc000330b40, 0x478ce80, 0xc0001afbc0)
        github.com/prometheus/node_exporter/collector/collector.go:153 +0x84
github.com/prometheus/node_exporter/collector.NodeCollector.Collect.func1(0xc000330b40, 0xc0001afe60, 0x478ce80, 0xc0001afbc0, 0xc0003a13b0, 0x46d28ac, 0x9, 0x478d080, 0xc00028c330)
        github.com/prometheus/node_exporter/collector/collector.go:144 +0x71
created by github.com/prometheus/node_exporter/collector.NodeCollector.Collect
        github.com/prometheus/node_exporter/collector/collector.go:143 +0x13c

lufia
lufia reviewed Sep 2, 2021
View reviewed changes
@lufia
Copy link
Owner

lufia commented Sep 2, 2021

16 disks! great!

@VladRassokhin
Copy link
Contributor Author

VladRassokhin commented Sep 2, 2021 •
edited
Loading

16 disks! great!

Most of that disks are virtual, also problem could be in other place. Anyway program should not panic in such case.

@lufia
Copy link
Owner

lufia commented Sep 2, 2021

@VladRassokhin thanks. I will release new version soon.

@lufia lufia merged commit fccb6ed into lufia:master Sep 2, 2021
@lufia
Copy link
Owner

lufia commented Sep 2, 2021

released: https://pkg.go.dev/github.com/lufia/iostat@v1.1.1

@VladRassokhin
Copy link
Contributor Author

VladRassokhin commented Sep 2, 2021

Thank you!

VladRassokhin added a commit to VladRassokhin/node_exporter that referenced this pull request Sep 2, 2021
@VladRassokhin
Update github.com/lufia/iostat to v1.1.1
977996b 
New version contains fix for `panic`, see lufia/iostat#2

Signed-off-by: Vladislav Rassokhin <vladislav.rassokhin@jetbrains.com>
@VladRassokhin VladRassokhin mentioned this pull request Sep 2, 2021
Merged
@VladRassokhin VladRassokhin deleted the dont-panic branch September 2, 2021 11:36
discordianfish pushed a commit to prometheus/node_exporter that referenced this pull request Sep 2, 2021
@VladRassokhin @discordianfish
Update github.com/lufia/iostat to v1.1.1
647e846 
New version contains fix for `panic`, see lufia/iostat#2

Signed-off-by: Vladislav Rassokhin <vladislav.rassokhin@jetbrains.com>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
@VladRassokhin @oblitorum
Update github.com/lufia/iostat to v1.1.1
7decd75 
New version contains fix for `panic`, see lufia/iostat#2

Signed-off-by: Vladislav Rassokhin <vladislav.rassokhin@jetbrains.com>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
@VladRassokhin @oblitorum
Update github.com/lufia/iostat to v1.1.1
e1670b0 
New version contains fix for `panic`, see lufia/iostat#2

Signed-off-by: Vladislav Rassokhin <vladislav.rassokhin@jetbrains.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lufia lufia lufia left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@VladRassokhin @lufia