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

fix getOutput, collect all buffer for sync process #9948

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@rudych
Copy link

rudych commented Mar 13, 2019

fix getOutput, collect all buffer.

before changed, when get graph, got red bar, because getOutput is not completed.
output of debug graph.php
https://x.y.z/graph.php?height=45&debug=1

SQL[SELECT `devices`.*, `location`, `lat`, `lng` FROM `devices` LEFT JOIN locations ON `devices`.location_id=`locations`.`id` WHERE `device_id` = ? ["1"] 0.78ms] SQL[SELECT * FROM devices_attribs WHERE `device_id` = ? [1] 0.3ms] SQL[SELECT * FROM `vrf_lite_cisco` WHERE `device_id` = ? ["1"] 0.32ms] SQL[SELECT attrib_value FROM devices_attribs WHERE `device_id` = ? AND `attrib_type` = ? [1,"poll_mib"] 0.29ms] SQL[SELECT * FROM `processors` where `device_id` = ? [1] 2.95ms] RRD[last 10.10.0.1/processor-hr-196608.rrd --daemon rrdcached:42217] RRD[last 10.10.0.1/processor-hr-196609.rrd --daemon rrdcached:42217] 
Warning: substr_count(): Invalid length value in /opt/librenms/includes/rrdtool.inc.php on line 284
graph /tmp/xEaKh3bU2U7yrExX -g -l 0 -u 100 -E --start 1552416900 --end 1552503300 --width 150 --height 45 -c BACK#EEEEEE00 -c SHADEA#EEEEEE00 -c SHADEB#EEEEEE00 -c FONT#000000 -c CANVAS#FFFFFF00 -c GRID#a5a5a5 -c MGRID#FF9999 -c FRAME#5e5e5e -c ARROW#5e5e5e -R normal -c CANVAS#FFFFFF00 --only-graph --font LEGEND:7:DejaVuSansMono --font AXIS:6:DejaVuSansMono --font-render-mode normal COMMENT:'Load % Now Min Max Avg\l' DEF:usage0=10.10.0.1/processor-hr-196608.rrd:usage:AVERAGE DEF:usage0min=10.10.0.1/processor-hr-196608.rrd:usage:MIN DEF:usage0max=10.10.0.1/processor-hr-196608.rrd:usage:MAX CDEF:usage_cdef0=usage0,2,/ CDEF:usage_cdef0min=usage0min,2,/ CDEF:usage_cdef0max=usage0max,2,/ AREA:usage_cdef0#E43C00:'Intel Xeon E5-26 ' GPRINT:usage0:LAST:%5.2lf%s GPRINT:usage0min:MIN:%5.2lf%s GPRINT:usage0max:MAX:%5.2lf%s GPRINT:usage0:AVERAGE:'%5.2lf%s\n' COMMENT:'\n' DEF:usage1=10.10.0.1/processor-hr-196609.rrd:usage:AVERAGE DEF:usage1min=10.10.0.1/processor-hr-196609.rrd:usage:MIN DEF:usage1max=10.10.0.1/processor-hr-196609.rrd:usage:MAX CDEF:usage_cdef1=usage1,2,/ CDEF:usage_cdef1min=usage1min,2,/ CDEF:usage_cdef1max=usage1max,2,/ AREA:usage_cdef1#E74B00:'Intel Xeon E5-26 ':STACK GPRINT:usage1:LAST:%5.2lf%s GPRINT:usage1min:MIN:%5.2lf%s GPRINT:usage1max:MAX:%5.2lf%s GPRINT:usage1:AVERAGE:'%5.2lf%s\n' COMMENT:'\n' --daemon rrdcached:42217

command returned (1552503300 OK u:0.01 s:0.00 r:0.00 )

graph /tmp/xEaKh3bU2U7yrExX -g --alt-autoscale-max --rigid -E --start 1552416900 --end 1552503300 --width 150 --height 45 -c BACK#EEEEEE00 -c SHADEA#EEEEEE00 -c SHADEB#EEEEEE00 -c FONT#000000 -c CANVAS#FFFFFF00 -c GRID#a5a5a5 -c MGRID#FF9999 -c FRAME#5e5e5e -c ARROW#5e5e5e -R normal -c CANVAS#FFFFFF00 --only-graph --font LEGEND:7:DejaVuSansMono --font AXIS:6:DejaVuSansMono --font-render-mode normal HRULE:0#555555 --title='Draw Error' --daemon rrdcached:42217

command returned (150x45 OK u:0.01 s:0.00 r:0.01 )

after changed, getOutput always get completed, graph is normal.

SQL[SELECT `devices`.*, `location`, `lat`, `lng` FROM `devices` LEFT JOIN locations ON `devices`.location_id=`locations`.`id` WHERE `device_id` = ? ["1"] 0.78ms] SQL[SELECT * FROM devices_attribs WHERE `device_id` = ? [1] 0.3ms] SQL[SELECT * FROM `vrf_lite_cisco` WHERE `device_id` = ? ["1"] 0.3ms] SQL[SELECT attrib_value FROM devices_attribs WHERE `device_id` = ? AND `attrib_type` = ? [1,"poll_mib"] 0.28ms] SQL[SELECT * FROM `processors` where `device_id` = ? [1] 1.16ms] RRD[last 10.10.0.1/processor-hr-196608.rrd --daemon rrdcached:42217] RRD[last 10.10.0.1/processor-hr-196609.rrd --daemon rrdcached:42217] 
Warning: substr_count(): Invalid length value in /opt/librenms/includes/rrdtool.inc.php on line 284
graph /tmp/z2YLFtVAH7ZtRzF9 -g -l 0 -u 100 -E --start 1552416900 --end 1552503300 --width 150 --height 45 -c BACK#EEEEEE00 -c SHADEA#EEEEEE00 -c SHADEB#EEEEEE00 -c FONT#000000 -c CANVAS#FFFFFF00 -c GRID#a5a5a5 -c MGRID#FF9999 -c FRAME#5e5e5e -c ARROW#5e5e5e -R normal -c CANVAS#FFFFFF00 --only-graph --font LEGEND:7:DejaVuSansMono --font AXIS:6:DejaVuSansMono --font-render-mode normal COMMENT:'Load % Now Min Max Avg\l' DEF:usage0=10.10.0.1/processor-hr-196608.rrd:usage:AVERAGE DEF:usage0min=10.10.0.1/processor-hr-196608.rrd:usage:MIN DEF:usage0max=10.10.0.1/processor-hr-196608.rrd:usage:MAX CDEF:usage_cdef0=usage0,2,/ CDEF:usage_cdef0min=usage0min,2,/ CDEF:usage_cdef0max=usage0max,2,/ AREA:usage_cdef0#E43C00:'Intel Xeon E5-26 ' GPRINT:usage0:LAST:%5.2lf%s GPRINT:usage0min:MIN:%5.2lf%s GPRINT:usage0max:MAX:%5.2lf%s GPRINT:usage0:AVERAGE:'%5.2lf%s\n' COMMENT:'\n' DEF:usage1=10.10.0.1/processor-hr-196609.rrd:usage:AVERAGE DEF:usage1min=10.10.0.1/processor-hr-196609.rrd:usage:MIN DEF:usage1max=10.10.0.1/processor-hr-196609.rrd:usage:MAX CDEF:usage_cdef1=usage1,2,/ CDEF:usage_cdef1min=usage1min,2,/ CDEF:usage_cdef1max=usage1max,2,/ AREA:usage_cdef1#E74B00:'Intel Xeon E5-26 ':STACK GPRINT:usage1:LAST:%5.2lf%s GPRINT:usage1min:MIN:%5.2lf%s GPRINT:usage1max:MAX:%5.2lf%s GPRINT:usage1:AVERAGE:'%5.2lf%s\n' COMMENT:'\n' --daemon rrdcached:42217

command returned (150x45 OK u:0.01 s:0.00 r:0.04 )

-rw-rw-r-- 1 librenms librenms 2165 Mar 14 01:56 /tmp/z2YLFtVAH7ZtRzF9 graph
Runtime 0.079s
SNMP [0/0.00s]: Get[0/0.00s] Getnext[0/0.00s] Walk[0/0.00s] MySQL [5/0.00s]: Cell[0/0.00s] Row[2/0.00s] Rows[-1/0.00s] Column[4/0.00s] Update[0/0.00s] Insert[0/0.00s] Delete[0/0.00s] RRD [2/0.04s]: Update[0/0.00s] Create [0/0.00s] Other[2/0.04s]

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Mar 13, 2019

CLA assistant check
All committers have signed the CLA.

@rudych
Copy link
Author

rudych left a comment

change break to conditional var

@rudych
Copy link
Author

rudych left a comment

fix issue code climate

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Mar 14, 2019

@rudych Thanks for the PR. We are generally trying to move away from the custom Proc class and move to Symfony Process https://symfony.com/doc/current/components/process.html

@rudych
Copy link
Author

rudych left a comment

getOutput, collect all buffer for sync only

@rudych rudych changed the title - fixed getOutput, collect all buffer fix getOutput, collect all buffer for async process Mar 14, 2019

@rudych rudych changed the title fix getOutput, collect all buffer for async process fix getOutput, collect all buffer for sync process Mar 14, 2019

@murrant
Copy link
Member

murrant left a comment

I'm not sure what this fixes and it seems to skip a check. Maybe that is how it "fixes" your issue.

I'm also not sure this won't break other things. So if we are going to break things, we should just port this to Symfony Process.

@rudych

This comment has been minimized.

Copy link
Author

rudych commented Mar 19, 2019

as i mention in first comment, getOutput not return in complete output for sync process rrdtool.

RRD[last 10.10.0.1/processor-hr-196608.rrd --daemon rrdcached:42217]
RRD[last 10.10.0.1/processor-hr-196609.rrd --daemon rrdcached:42217]
graph /tmp/xEaKh3bU2U7yrExX ... rrdcached:42217
command returned (1552503300 OK u:0.01 s:0.00 r:0.00 )

graph /tmp/xEaKh3bU2U7yrExX ... rrdcached:42217
command returned (150x45 OK u:0.01 s:0.00 r:0.01 )

before fixes, first "graph" return "1552503300", which is buffer from "last" cmd.
because graph process is not finished, it fallback to red bar second "graph".

after fixes, first "graph" will make sure buffer is completed.
graph process is finished, and display properly.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Mar 19, 2019

Shouldn't there be one lastupdate call and one graph call? As the first one shows? Not two graph calls?

Also, what is your setup to reproduce this issue?

Sorry, for asking so many questions, but this code change could affect a lot of things.

@laf laf added the User-Pending label Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.