etingof / pysnmp

Python SNMP library
http://snmplabs.com/pysnmp/
BSD 2-Clause "Simplified" License
568 stars 188 forks source link

Using timeouts lower than 0.5 secs #250

Open nadavtm opened 5 years ago

nadavtm commented 5 years ago

Hi! I'm trying to use timeouts lower than 0.5 secs (let's say 0.2). By default, the internal timer resolution is 0.5 so that it won't work out of the box. In the code below, after the first request the engine makes, it has a transport dispatcher and then I set the timer resolution to be 0.2. Looking a the debug log, we can see that the timer resolution changes from: pysnmp: sendPdu: current time 0 ticks, one tick is 0.5 seconds to: pysnmp: sendPdu: current time 1 ticks, one tick is 0.04 seconds but the actual waiting time still remains 0.5 before we get a timeout. I'm using Python 3.7 Any ideas? Thanks in advance!

import logging
from time import time
from pysnmp import hlapi, debug

def make_request(engine, oid, transport_target):
    error_indication, error_status, error_index, var_binds = next(
        hlapi.getCmd(engine,
                     hlapi.CommunityData('public'),
                     transport_target,
                     hlapi.ContextData(),
                     hlapi.ObjectType(hlapi.ObjectIdentity(oid)))
    )

    # client errors like timeouts will be here
    if error_indication:
        raise RuntimeError(error_indication)

    # agent-originated errors will appear here
    elif error_status:
        raise RuntimeError('%s at %s' % (error_status.prettyPrint(),
                                         error_index and var_binds[int(error_index) - 1][0] or '?'))
    else:
        var = var_binds[0][1]

        # last check to see that we got an actual value
        if isinstance(var, hlapi.NoSuchObject):
            raise RuntimeError(f'Object {oid} could not be found')
        return var.prettyPrint()

# turn on detailed logging
debug.setLogger(debug.Debug('all'))

# create our engine
engine = hlapi.SnmpEngine()
# sysDescr OID
oid = '1.3.6.1.2.1.1.1.0'
# timeout lower than 0.5
timeout = 0.2
# send requests to localhost so we don't get a response
# since we are interested in a timeout
transport_target = hlapi.UdpTransportTarget(('127.0.0.1', 161), timeout=timeout, retries=0)

# make 5 sequential requests
for i in range(5):
    now = time()
    try:
        print(f'--- making request {i}')
        print(f'result {i}: {make_request(engine, oid, transport_target) + str(i)}')
    except:
        logging.exception(f'an error occurred on request {i}')
    finally:
        # if this is after the first request, we can set the timer resolution
        # because now the engine has a transportDispatcher
        if i == 0:
            print('setting timer resolution')
            engine.transportDispatcher.setTimerResolution(timeout)
            engine.transportDispatcher.timeout = timeout
        print(f'--- request {i} ended. took {int((time() - now) * 1000)} ms')
etingof commented 5 years ago

Thank you for reporting this issue and sorry for taking time looking into it!

It seems like a bug (or a forgotten thing) in pysnmp. It's hopefully fixed In the release candidate.

If you happen to test this fix and give me a go-ahead, I'd make a release.

nadavtm commented 5 years ago

Thank you very much for the quick fix Ilya. Looks like it resolves the issue as I can now set timeouts lower than 0.5 secs and they are respected by the engine. I do have a couple questions:

  1. Just to make sure, there should not be any issue limiting the rate of requests by calling sleep() in between requests, right? It shouldn't mess up the internal timeout calculations or anything right?
  2. To get accurate results I set the timer resolution to 1/10 of the timeout setting I want. If it's closer to the timeout itself, I start getting major deviations from the timeout setting. Do you think it could pose a performance issue?
  3. I don't think it is required to set the self.timeout of AsyncoreDispatcher in addition to setTimerResolution() as I can see it now uses getTimerResolution() internally.
  4. Is there a way to set the timer resolution prior to the first request? Create an instance of the dispatcher with the desired config and pass it to engine? I didn't see any docs for that.

Thanks!

etingof commented 5 years ago
  1. Just to make sure, there should not be any issue limiting the rate of requests by calling sleep() in between requests, right? It shouldn't mess up the internal timeout calculations or anything right?

I believe the timing is being computed relative to time() so sleep()'ing should be alright.

  1. To get accurate results I set the timer resolution to 1/10 of the timeout setting I want. If it's closer to the timeout itself, I start getting major deviations from the timeout setting. Do you think it could pose a performance issue?

It's hard to say - depends on many things. You should see it yourself - do you observe any resource shortage or excessive use?

BTW, if you have concerns about performance and do not plan to use SNMPv3, there is the lighter alternative known as hlapy/v1arch. It's only available in pysnmp 5 which is being in development.

  1. I don't think it is required to set the self.timeout of AsyncoreDispatcher in addition to setTimerResolution() as I can see it now uses getTimerResolution() internally.

Right, let me revisit that part...

  1. Is there a way to set the timer resolution prior to the first request? Create an instance of the dispatcher with the desired config and pass it to engine? I didn't see any docs for that.

I do not see an easy way as of the current hlapi code. You can do that with the native v1arch and v3arch though.

I am thinking that possibly the cleanest solution could be to automatically reduce timer resolution based on the minimum retry timeout encountered among all the targets...?

WDYT?

nadavtm commented 5 years ago
  1. Just to make sure, there should not be any issue limiting the rate of requests by calling sleep() in between requests, right? It shouldn't mess up the internal timeout calculations or anything right?

I believe the timing is being computed relative to time() so sleep()'ing should be alright.

I was referring to this (in handleTimerTick()), not sure if it's a problem: self.__nextTime = timeNow + self.__timerResolution - self.__timerDelta

  1. To get accurate results I set the timer resolution to 1/10 of the timeout setting I want. If it's closer to the timeout itself, I start getting major deviations from the timeout setting. Do you think it could pose a performance issue?

It's hard to say - depends on many things. You should see it yourself - do you observe any resource shortage or excessive use?

I don't see anything unusual from CPU/memory perspective so I think it's ok.

BTW, if you have concerns about performance and do not plan to use SNMPv3, there is the lighter alternative known as hlapy/v1arch. It's only available in pysnmp 5 which is being in development.

I do need v3 support but I will check the new APIs once pysnmp 5 is released.

  1. I don't think it is required to set the self.timeout of AsyncoreDispatcher in addition to setTimerResolution() as I can see it now uses getTimerResolution() internally.

Right, let me revisit that part...

  1. Is there a way to set the timer resolution prior to the first request? Create an instance of the dispatcher with the desired config and pass it to engine? I didn't see any docs for that.

I do not see an easy way as of the current hlapi code. You can do that with the native v1arch and v3arch though.

I am thinking that possibly the cleanest solution could be to automatically reduce timer resolution based on the minimum retry timeout encountered among all the targets...?

WDYT?

Well, I can see a couple of issues with automatically setting the resolution. Firstly, after changing the resolution, it could take a few requests for things to get smooth and timely (depends on the exact numbers for timeout, resolution and sleep period and what they were before) because let's say the resolution was 1 hour, changing it to 1 second will not happen until the next tick, which could be in up to an hour. Also, I would want to be able to set the resolution to be smaller (maybe much smaller, possibly 1/10) than the timeout setting, the wanted ratio could be different to accommodate different precision requirements. And finally, I would want it to take effect starting from the first request.

Is there a way to pass it to the engine's constructor which in turn will pass it to the dispatcher? Could we pass it to getCmd that will pass it down?

-------- EDIT ---------- Or possibly, maybe easiest to implement is a module-level/global var/class member to override with the wanted timer resolution prior to creating any engines or making any requests and the default sensitivity will be taken from that if set, else the default.

lextm commented 4 months ago

The timeout mechanism used in asyncore implementation was discussed above.

However, once fully migrated to asyncio and use self.loop.call_later properly, the timeout mechanism can be quite simple.

https://github.com/lextudio/pysnmp/blob/v5.0.36/pysnmp/carrier/asyncio/dispatch.py#L66

The upgraded sample code only needs the timeout value to be passed to runDispatcher,

https://github.com/lextudio/pysnmp/blob/v5.0.36/examples/v3arch/asyncio/manager/cmdgen/v1-get.py#L84