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

Avoid redundant port counter collection from virtualized instances, such as Cisco VRF #1492

Merged
merged 35 commits into from Jun 8, 2017

Conversation

lunkwill42
Copy link
Member

@lunkwill42 lunkwill42 commented Mar 24, 2017

Fixes #1489

This pull request is not complete yet. A few adjustments are needed on the UI after feedback, and the entire functionality should be reviewed by @mdiehm.

lunkwill42 and others added 12 commits March 21, 2017 08:11
i.e. in the case of VRF, the physical node is the master. Other modes of
operation that are similar to VRF exist (including VDC).
…ances

In the case of a master netbox, duplicate all collected port metrics
across all instance netboxes when shipping to Graphite.

In the case of a virtualized instance, just debug log a list of local
ports (that do not appear on the master) and exit.

There is _no_ collection of data from local ports at the moment, as this
would require a complete restructuring of things (i.e collection from
individual ports, rather than full column snmpwalks for the values we
want)
instead, have the eventengine make copies of linkState events for master
devices.
@lunkwill42 lunkwill42 self-assigned this Mar 24, 2017
John-Magne Bredal and others added 16 commits March 24, 2017 15:22
i.e. in the case of VRF, the physical node is the master. Other modes of
operation that are similar to VRF exist (including VDC).
…ances

In the case of a master netbox, duplicate all collected port metrics
across all instance netboxes when shipping to Graphite.

In the case of a virtualized instance, just debug log a list of local
ports (that do not appear on the master) and exit.

There is _no_ collection of data from local ports at the moment, as this
would require a complete restructuring of things (i.e collection from
individual ports, rather than full column snmpwalks for the values we
want)
instead, have the eventengine make copies of linkState events for master
devices.
@jmbredal
Copy link
Collaborator

Please pull my latest changes from https://github.com/jmbredal/nav/tree/vrf

@mdiehm
Copy link

mdiehm commented May 11, 2017

Reduce the number of redundant polls

To achieve that how about:

  • no need for duplicate sensons polling (cpu/module/..)
  • no need for cdp neigh et al discovery (neighbours) on the virt. host (it’s all the same within one vdc)
  • what else?

I tried to identify the biggest gain on deduplication of specific
other snmp-requests but couldn’t find a clear winner. So I hope
removing the above mentioned gives us another big drop in snmp
requests. I really think in a long term we should try to deduplicate as
much as possible. Could you please go ahaid and try to implement that?

MB: I agree, we should probably do the same for the 1minstats job, as that
can sometimes be more intensive than 5minstats.

@mdiehm
Copy link

mdiehm commented May 11, 2017

Would be nice to have more visibility of the Role a device has (Master/virt.Device) on the general overviewpages like the ones attached.

image

image

sigmunau and others added 3 commits May 31, 2017 13:39
@lunkwill42
Copy link
Member Author

So, @mdiehm, @sigmunau has made changes to the statsystem and statsensors plugins, which run in the 1minstats job. Nothing has been done in regard to topology collection thus far. Topology data is collected only in 15 minute intervals, and to me it sounds quite uncertain what the effects would be of changing the collection behavior here, without having a proper VRF setup to test on.

I will look into making some UI changes, as requested above, but in the meantime, a new Debian packages is being built at https://ci.nav.uninett.no/job/dpkg-build-vrf/ as usual

SeedDB is, IMNSHO, overdue for a rewrite. The way it's written makes it
really hard to do anything useful outside of building tables of text
strings.

This hack will at least enable any custom templates to access the
original model object of each row, instead of a list of dicts containing
compressed attribute strings :-P
Implemented as a custom template overriding the row logic for netbox
lists.
@lunkwill42
Copy link
Member Author

lunkwill42 commented Jun 2, 2017

SeedDB now looks something like this:

seeddb with labels and title

It was harder than I thought to make SeedDB do this. It will be even harder to do it with the search results without rewriting the entire search result rendering system. Awaiting further instructions.

@lunkwill42
Copy link
Member Author

A new release is upon us this week.

All committed changes to the PR work as a unit, and should not break existing functionality. We will therefore merge the PR to master before the release. Any further changes to the UI or polling plugins should go in a separate PR after the release (which means any further work is not likely to appear in 4.7, but 4.8).

Copy link
Collaborator

@jmbredal jmbredal left a comment

Choose a reason for hiding this comment

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

Not much to comment on, I have only briefly gone through the ipdevpoll-code and the rest looks perfectly fine. One very minor thing that optionally can be done.

Should conform to the bassackwards ways of SeedDB at large.
"""
try:
value = reduce(getattr, attr.split('__'), obj)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps make it Python 3 compatible
http://python-future.org/compatible_idioms.html#reduce

Copy link
Member Author

Choose a reason for hiding this comment

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

will do, after merging :)

@lunkwill42 lunkwill42 merged commit 241f001 into Uninett:master Jun 8, 2017
@lunkwill42 lunkwill42 deleted the vrf branch June 8, 2017 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants