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.

Sign up for GitHub

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

Update varnish5_ improving request_rate, transfer_rates #1304

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update varnish5_ improving request_rate, transfer_rates #1304

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 32 additions & 32 deletions plugins/varnish/varnish5_
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,29 +170,35 @@ my %data;
my %ASPECTS = (
'request_rate' => {
'title' => 'Request rates',
'order' => 'cache_hit cache_hitpass cache_miss cache_hitmiss'
. 'backend_conn backend_unhealthy '
. 'client_req client_conn' ,
'order' => 'sess_conn client_req s_pass cache_miss cache_hitmiss cache_hitpass cache_hit s_pipe'
. ' backend_conn backend_unhealthy',
'values' => {
'sess_conn' => {
'type' => 'DERIVE',
'draw' => 'AREA',
'min' => '0',
'colour' => '444444',
'colour' => 'FF5D00',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these color changes just your personal preference or objective improvements?
In case of doubt, I would prefer to keep them unchanged.

'graph' => 'ON'
},
'client_req' => {
'type' => 'DERIVE',
'colour' => '111111',
'min' => '0'
'colour' => '777777',
'min' => '0',
},
'cache_hit' => {
's_pass' => {
'type' => 'DERIVE',
'draw' => 'AREA',
'colour' => '00FF00',
'min' => '0',
'colour' => 'BA4300'
},
'cache_miss' => {
'type' => 'DERIVE',
'colour' => 'FF0000',
'draw' => 'STACK',
'min' => '0'
},
'cache_hitpass' => {
'info' => 'Hitpass are cached passes: An '
'cache_hitmiss' => {
'info' => 'Hitmiss are cached missing: An '
. 'entry in the cache instructing '
. 'Varnish to pass. Typically '
. 'achieved after a pass in '
Expand All @@ -202,14 +208,8 @@ my %ASPECTS = (
'colour' => 'FFFF00',
'min' => '0'
},
'cache_miss' => {
'type' => 'DERIVE',
'colour' => 'FF0000',
'draw' => 'STACK',
'min' => '0'
},
'cache_hitmiss' => {
'info' => 'Hitmiss are cached missing: An '
'cache_hitpass' => {
'info' => 'Hitpass are cached passes: An '
. 'entry in the cache instructing '
. 'Varnish to pass. Typically '
. 'achieved after a pass in '
Expand All @@ -219,25 +219,27 @@ my %ASPECTS = (
'colour' => 'FFFF00',
'min' => '0'
},
'backend_conn' => {
'cache_hit' => {
'type' => 'DERIVE',
'colour' => '995599',
'draw' => 'STACK',
'colour' => '00FF00',
'min' => '0'
},
'backend_unhealthy' => {
's_pipe' => {
'type' => 'DERIVE',
'draw' => 'STACK',
'min' => '0',
'colour' => 'FF55FF'
'colour' => '0A7B0B'
},
's_pipe' => {
'backend_conn' => {
'type' => 'DERIVE',
'min' => '0',
'colour' => '1d2bdf'
'colour' => 'EEEEEE',
'min' => '0'
},
's_pass' => {
'backend_unhealthy' => {
'type' => 'DERIVE',
'min' => '0',
'colour' => '785d0d'
'colour' => 'FF55FF'
}
}
},
Expand Down Expand Up @@ -346,22 +348,22 @@ my %ASPECTS = (
},
'transfer_rates' => {
'title' => 'Transfer rates',
'order' => 's_resp_bodybytes s_resp_hdrbytes',
'order' => 's_resp_hdrbytes s_resp_bodybytes',
'args' => '-l 0',
'vlabel' => 'bit/s',
'values' => {
's_resp_hdrbytes' => {
'type' => 'DERIVE',
'label' => 'Header traffic',
'draw' => 'STACK',
'draw' => 'AREA',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this change intentional? Or did you mean AREASTACK?

Copy link
Author

Choose a reason for hiding this comment

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

Hi,

I had made a mail explaining a bit more what I did here and I had sent it to the last person what had maintained varnish5_ @kjetilho

On request_rates the change of color is required, the changes are far deeper than color changes.
The graph is the oposite of what it was so it would be posible to visualize passed and missed requests. Instead of having them stacked on top of hits, here I stack hits (big figure) on top of passed and missed (small numbers)

transfer_rates, in the same logic, lets have small numbers to be the first ones stacked, big ones on top.

having small figures stacked on top of big ones (like cache hits) makes them impossible to visualise. Hit rate can go from 1 to 200 in the course of a day, if one stacks on top something that varies between 0.5 and 3, the frange it creates on top of huge mountains isn't readable, and in practice it's the passed and miss we want to monitor, not the hits (hits aren't painful to backend and the hit-rate graph gives us what we need to know on hits, over time one wants to keep an eye on the evolution over time of 'pass' and 'miss')

Perhaps you want to have a look at the result
low trafic
https://munin.adm.ofil.fr/localdomain/localhost.localdomain/varnish5_request_rate.html
https://munin.adm.article3.fr/localdomain/localhost.localdomain/varnish5_request_rate.html

more serious trafic
https://munin.adm.welovetennis.fr/localdomain/localhost.localdomain/varnish5_request_rate.html

Copy link
Author

@cantoute cantoute Mar 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Is this change intentional? Or did you mean AREASTACK?

not sure I'm getting your question.
the plugin used AREA to define first graph and STACK for those that pile on top.
I just swapped them. The header traffic is very small compared to body, stacking it on top makes the graph useless.

Did I miss something ?

Copy link
Author

Choose a reason for hiding this comment

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

hi,
Looks like I had miss-understood sess_conn and working at fixing this.

I'll need a bit more test time and will come back with a new pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the plugin used AREA to define first graph and STACK for those that pile on top.

Maybe it helps to simplify the code: you can also use AREASTACK for all fields. The old distinction between the first and the following fields is not necessary anymore.

I'll need a bit more test time and will come back with a new pull request.

Feel free to recycle this PR by force-pushing into it. Or just close this one and open a new one.
Thanks!

'min' => '0',
'info' => 'HTTP Header traffic. TCP/IP '
. 'overhead is not included.',
'cdef' => 's_resp_hdrbytes,8,*'
},
's_resp_bodybytes' => {
'type' => 'DERIVE',
'draw' => 'AREA',
'draw' => 'STACK',
'label' => 'Body traffic',
'min' => '0',
'cdef' => 's_resp_bodybytes,8,*'
Expand Down Expand Up @@ -773,7 +775,6 @@ my %ASPECTS = (
},
'sess_pipe_overflow' => {
'type' => 'DERIVE'

}
}
},
Expand Down Expand Up @@ -1219,4 +1220,3 @@ foreach my $value (keys %{$ASPECTS{$self}{'values'}}) {
print "$data{$value}{'value'}\n";
}
}