Skip to content
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

Add zero minimum in linux power supply module #5395

Merged
merged 1 commit into from Mar 6, 2019

Conversation

@vlvkobal
Copy link
Member

vlvkobal commented Feb 14, 2019

Summary

Add a zero empty/min dimension when it is undefined but the corresponding full/max value exists.

Fixes missing zero dimensions in linux power supply module.

Component Name

proc.plugin

@vlvkobal vlvkobal requested a review from ktsaou as a code owner Feb 14, 2019
@vlvkobal vlvkobal requested a review from Ferroin Feb 14, 2019
@vlvkobal

This comment has been minimized.

Copy link
Member Author

vlvkobal commented Feb 14, 2019

@Ferroin please test it.

@@ -215,6 +222,16 @@ int do_sys_class_power_supply(int update_every, usec_t dt) {
pr->property_dim_root = pd;
}
}

// create a zero empty/min dimension
if(unlikely(max_value_found && !min_value_found)) {

This comment has been minimized.

Copy link
@Ferroin

Ferroin Feb 15, 2019

Collaborator

I'm not entirely sure that this should be marked as unlikely. Most of the systems I've seen actually are missing at least one lower bound dimension (usually the *_min_design or *_empty_design dimensions).

I'm not entirely sure it shouldn't be marked unlikely either though, given that it only really affects startup of the collector, and it shouldn't affect things much.

@@ -258,36 +275,38 @@ int do_sys_class_power_supply(int update_every, usec_t dt) {
for(pr = ps->property_root; pr && !read_error; pr = pr->next) {
struct ps_property_dim *pd;
for(pd = pr->property_dim_root; pd; pd = pd->next) {
char buffer[30 + 1];
if(likely(!pd->always_zero)) {

This comment has been minimized.

Copy link
@Ferroin

Ferroin Feb 15, 2019

Collaborator

This, on the other hand, makes sense to be marked as likely, because most of the systems I have seen have significantly more correct (not-always-zero) dimensions.

@Ferroin

This comment has been minimized.

Copy link
Collaborator

Ferroin commented Feb 15, 2019

@vlvkobal I probably won't have time to test this today, I'll try to get it tested this weekend. I've added a couple of comments to the code itself, but other than those, it looks generally correct to me.

Copy link
Collaborator

Ferroin left a comment

Appears to be working correctly here.

@vlvkobal vlvkobal requested a review from cakrit Mar 5, 2019
@ktsaou
ktsaou approved these changes Mar 6, 2019
@ktsaou

This comment has been minimized.

Copy link
Member

ktsaou commented Mar 6, 2019

As we discussed this module has a few issues to improve maintainability.
@cakrit we can merge this, but it needs some cleanup.

@vlvkobal vlvkobal merged commit c7ddf8d into netdata:master Mar 6, 2019
12 checks passed
12 checks passed
Header rules - netdata No header rules processed
Details
Pages changed - netdata 2 new files uploaded
Details
Redirect rules - netdata No redirect rules processed
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
Mixed content - netdata No mixed content detected
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details
netlify/netdata/deploy-preview Deploy preview ready!
Details
@vlvkobal vlvkobal deleted the vlvkobal:power-supply-min branch Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.