Reduce macsuck bandwidth usage to database #680
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
get_port_macs transfers a full list of all device_port.(mac,ip) in every macsuck. With 8k devices and 40k interfaces it takes up around 15MB. Transferring them 8k times during an 1h macsuck cycle requires bandwidth in the 300 to 400 mbit/s range. This patch changes get_port_macs to be called inside walk_fwtable and only transfer the macs found in the current target device/vlan.
|
at first glance seems to work just fine, but i'll need to wait a few days so i actually have users moving around etc... |
|
Some related RRD: Traffic before/after on database server switch port (hard to see now but there are some green pixels there)
Effect on poller performance
|
|
Performance implications of this PR from a single test over 7k devices, mostly Cisco:
Full CSV this comes from: |
rc9000
added a commit
that referenced
this pull request
Jan 26, 2020
Hi @ollyg! Unfortunately I have found some issues with the code we finally released in #680: * get_port_macs expects an array ref but values() returns array, so the code was never called due to the return unless... check * When fw_mac_list had exactly two entries, only the second value was bound as a scalar to the parameter. This is probably due to the shorthand bind formats described in https://metacpan.org/pod/DBIx::Class::ResultSet#DBIC-BIND-VALUES, but I'm not a 100% on this. * return unless now checks for an entry in the list, with the old check the statement was also executed for empty lists In cases where only the device(_port)?.mac lookup worked for uplink detection, users of 02.044005 - 010 might get a lot of uplinks not labeled as such.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From IRC today:
This PR tries to fix the above with a get_port_macs that's called inside walk_fwtable and only returns the macs found in the current target device/vlan.
If I'm reading walk_fwtable right the only lookups that can ever be made are for macs that the current snmp->fw_mac contains, but I'd appreciate if @ollyg and other cracks could verify this assumption. Also, I did not find usages of get_port_macs outside of Macsuck/Nodes.pm, so I hope this doesn't break anything.
I'm trying it now in our environment and it both seems to work ok so far and has reduced bandwidth DB -> poller by orders of magnitude.