snimpy icon indicating copy to clipboard operation
snimpy copied to clipboard

Table traversal

Open ghost opened this issue 9 years ago • 19 comments

Not a real issue, just some notes to table traversal: (1) the recipe for table traversal in documentation is wrong (very inefficient):

for index in manager.ifDescr:
    print(manager.ifDescr[index]) # results in one more (useless) SNMP get
                                  # for every iteration

it shoud be rewritten (should work in head, but tested only with 0.8.3) as:

for index, ifDescr in manager.ifDescr.iteritems():
    print(ifDescr)

or without the index if you also implement a ProxyColumn.itervalues(), (2) in last versions, one is allowed to change the default bulk parameters, but I would propose to change them also "per traversal" - like .iteritems(bulk = False) or .iteritems(max_bulk = 10) - it is quite usefull, (3) why does the .iteritems() method walk all values before yielding first item? it could yield first value(s) after first (bulk) response -- this would not only speed up first results, but would be also more efficient if the caller does not consume all the values (i.e. break in the mentioned for ... constructs).

ghost avatar Nov 30 '15 23:11 ghost

Hey!

The main goal of Snimpy is to feel "pythonic". The use of iteritems() is "less" pythonic. But I agree this is less efficient. There is also caching that could help here.

As for iteritems, I don't remember why I implement that instead of doing all that in iter. I may need to dig a bit about that. Depending on that, adding a bulk parameter would be possible or not.

For (3), which version of PySNMP are you using?

vincentbernat avatar Dec 01 '15 09:12 vincentbernat

Hello Vincent,

I understand and agree you've chosen MIB "columns" to be accessible like Python dicts. I just proposed to document how to write the traversal more efficiently (the historical coincidence Python dicts iterate over keys by default, which causes Snimpy iterator to fetch again the same values it has just received -- I agree your current API is more pythonic, its right).

I believe you implemented .iteritems() just as a more efficient alternative to .__iter__() :-) I noticed this behavior (the additional SNMP get(s) when traversing with default iterator as in the readthedocs) by an accident (wasn't looking for efficiency): I have a buggy agent (SNMP card for a UPS) which returns tabular data only to 'get next' (is not able to send a table item in response to specific 'get').

I discovered (3) at first on my Debian stable: Python 2.7.9 + Snimpy 0.8.3. I've just tried with Debian testing as well: Python 3.4.3 + Snimpy 0.8.8. But it behaves the same: walks (SNMP bulk get next) all the values:

#!/usr/bin/python3
import snimpy.manager
snimpy.manager.load('IF-MIB')
m = snimpy.manager.Manager(..., version = 2)
for index, if_descr in manager.ifDescr.iteritems():
    break

ghost avatar Dec 01 '15 19:12 ghost

For (3), I would have believed that I should have yield each bulk get. I'll check. I know this is not the case anymore with PySNMP 4.3 which turns a GETBULK request into a complete walk, but if you are using Debian packages, PySNMP is not at this version yet.

For (2), I believe that this could be done with contexts instead through the use of with. This way, we wouldn't need to rely on the fact that iteritems() is not the standard Python function.

For (1), my solution would have been to use caching. I have updated the documentation in respect of that.

vincentbernat avatar Dec 01 '15 20:12 vincentbernat

For (3), you are right. While I yield values in the higher levels, in the lower layers, I am using the simple command generator in PySNMP and it doesn't seem to provide an iterator on the results with versions < 4.3.0. I'll either upgrade to 4.3 at some point or rollback to using NetSNMP (for independent reasons). I'll fix that at this moment.

vincentbernat avatar Dec 01 '15 20:12 vincentbernat

Thank you. Not only for this small change, but for the (idea of) Snimpy in general: I haven't found any "ORM" SNMP analogy for any scripting langugage yet, your library is my favorite as being nearest. Not as a toy to retrieve few statistical SNMP variables, but for a larger project to autom8 configuration of a carrier L2/L3 device: my friend ended up with entering tens of OIDs in a messy PHP script to achieve the same... Btw, do you have any idea/plan to GET multiple variables in a single request (SET is already possible in with: context)?

ghost avatar Dec 01 '15 21:12 ghost

The difficulty for that is to find a pythonic way to do that. Until now, I didn't find anything. This is one of the main feature of Snimpy that I want to keep "true".

vincentbernat avatar Dec 01 '15 22:12 vincentbernat

So, about iteritems(), there is still no "abstract" way to do that in Python (I mean, no __iteritems__() in the different abstract classes). So iteritems() is here to stay. I'll add contextual bulk parameters shortly.

vincentbernat avatar Dec 01 '15 23:12 vincentbernat

Sorry for touching this again: I found an unexpected behavior just now. When trying to traverse an empty column, at least with SNMPv1 (as tested), the Session._op() raises SNMPEndOfMibView(), which is not handled in the iterator: instead of returning no items (throwin StopIteration), it leaks the SNMPEndOfMibView. Tested with 0.8.3 (the mentioned Debian setup) and skimming through master version here didn't reveal any patch for this.

ghost avatar Dec 02 '15 21:12 ghost

Could you show the exception? I think I see where the problem is but can't look at it right now. The exception would help.

vincentbernat avatar Dec 02 '15 22:12 vincentbernat

Sure. Please note it is 0.8.3 so are the line numbers. The ups_snmp in my script is a Manager instance, SNMPv1, with standard UPS-MIB loaded.

Traceback (most recent call last):
  File "/usr/local/bin/eatonbtest", line 193, in <module>
    for idx, upsAlarmDescr in ups_snmp.upsAlarmDescr.iteritems():
  File "/usr/lib/python2.7/dist-packages/snimpy/manager.py", line 379, in iteritems
    for noid, result in self.session.walk(oid):
  File "/usr/lib/python2.7/dist-packages/snimpy/snmp.py", line 268, in walk
    return self._op(self._cmdgen.nextCmd, *oids)
  File "/usr/lib/python2.7/dist-packages/snimpy/snmp.py", line 246, in _op
    raise SNMPEndOfMibView("no more stuff after this OID")  # nopep8
SNMPEndOfMibView: no more stuff after this OID

tcpdump: 1x get-next-request (OID=upsAlarmDescr), 1x get-response (OID>upsAlarmDescr)

ghost avatar Dec 02 '15 22:12 ghost

OK, that should be easy to fix.

vincentbernat avatar Dec 02 '15 22:12 vincentbernat

I have pushed a8551d731e5e for that.

vincentbernat avatar Dec 03 '15 08:12 vincentbernat

Hi Vincent,

I'm right now confronted with the efficiency issue (thousands of devices, and many OID's per device), that's probably what I should call "success" :-)

Question: any idea when you will have contextual bulk parameters implemented ?

Thx, Charles

cbueche avatar Feb 11 '16 15:02 cbueche

I am currently swamped, so I can't give you any timeline on this.

vincentbernat avatar Feb 11 '16 16:02 vincentbernat

ok no problem, I can wait, thanks for your work Vincent !

cbueche avatar Feb 12 '16 08:02 cbueche

Hi,

As I understand ghost's request and Vincent's answer on 1.12.2015, there is currently no efficient / getbulk way of getting multiple "columns" for a table using a single iteritem() loop ?

My goal would be to have one loop over ifTable (1.3.6.1.2.1.2.2) and get a few values for each index, e.g. the following code produces a bulk-get (quick) and then loops over the ifMtu column, very "slowly".

for index, desc in m.ifDescr.iteritems():
    interface = {}
    interface['index'] = index
    interface['ifDescr'] = desc
    interface['ifMtu'] = m.ifMtu[index]

As I understand, the only way would be to to multiple distinct loops, one for each column I need, and then merge the results together. Or any other suggestion ?

Using pysnmp==4.3.2 and snimpy==0.8.10

Thx ! Charles

cbueche avatar Apr 14 '16 12:04 cbueche

@cbueche We could create a proxy object where we register several OID:

for index, desc, mtu in m.ifDescr.and.ifMtu.iteritems():
    ...

This is a bit more Ruby than Python.

vincentbernat avatar Apr 14 '16 12:04 vincentbernat

with the 7-8 columns I need, the line length might be indecent :-) Anyway, thx, I will implement multiple iteritems() and merge the results together.

cbueche avatar Apr 14 '16 15:04 cbueche

Well, you could do that in several times:

proxy = m.ifDescr
proxy &= m.ifMtu
proxy &= ...

More pythonic to just overload &.

vincentbernat avatar Apr 14 '16 16:04 vincentbernat