search

OpenPrinting / ipp-usb

ipp-usb -- HTTP reverse proxy, backed by IPP-over-USB connection to device
BSD 2-Clause "Simplified" License
129 stars 11 forks source link

PnPTimeout handling #31

Open fabled opened 3 years ago

fabled commented 3 years ago

const.go defines DevInitRetryInterval but it is never used. I assume pnp.go: PnPStart and pnpRetryTime should be using it instead of DNSSdRetryInterval.

fabled commented 3 years ago

If using https://usbguard.github.io/ or similar, and the device is unauthorized by default, the first attempt to open the device will typically fail. However, the whitelisting would likely happen fast. So perhaps doing the first retry of failed open attempt fairly soon (after 100ms or so) would be helpful. The following retries could be increased incrementally/exponentially, or just have two timeouts (first fast one, and second longer one).

alexpevzner commented 3 years ago

Probably, I've used DNSSdRetryInterval instead of DevInitRetryInterval by mistake. Fixed.

Regarding using small timeout for the first initialization attempt, I can implement it, but I'm not sure if it really needed. Existent code only delays printer initialization for a couple of seconds; printer is a "big thing", and I don't think somebody expects it to be available in a few milliseconds after connected. What do you think?

fabled commented 3 years ago

What do you think?

I mentioned it, because for me it was observable performance degradation when going from lp0 mode to ipp mode. The problem is that for IPP, there's so much happening creating longer delay. lp0 mode just adds the printer to cups from udev rule, and it becomes user visible in UI very fast. For IPP, mode the following things have to happen, all with some latency:

  1. IPP-USB detects and announces the printer (now with ~2s latency due to first failure)
  2. udev hook using waiting for the avahi announce
  3. udev hook querying printer serial to find the printer URI (could probably now avoid querying by using _ipp-usb._tcp records)
  4. Adding CUPS printer in driverless mode, which also queries printer capabilities

Given the whole chain there was several seconds more delay from printer plugin to becoming visible in UI. So I figured the 2s retry was first easy target to improve things a bit.

fabled commented 3 years ago

Then again, seems this is not too huge issue, and I can just adjust the DevInitRetryInterval on local build. The fix of using the proper const value in code, is good enough for me at this time for this issue. Thank you!

alexpevzner commented 3 years ago

DevInitRetryInterval is some arbitrary value, intuitively chosen as a reasonable trade-off between reaction time and power consumption/logs pollution, if device initialization repetitively fails. So I have no objections to adjust it within a reasonable range even in non-local builds.

You are also almost convinced me to implement exponential bake-off, starting from some small value.