Manishearth / ChatExchange

A Python API for talking to Stack Exchange chat
Apache License 2.0
65 stars 36 forks source link

Throttling, Retrying #69

Open banksJeremy opened 10 years ago

banksJeremy commented 10 years ago

As discussed in #59, we want to have some universal throttling mechanism for client requests.


Throttling

When we post a new message, or edit one, the requests are throttled, and retried when appropriate. This is good, but doesn't apply to any of the other requests we make. We should generalize the existing code, and make it easy to apply for different types of requests. (There would need to be some code specific to recognizing success/temporary error/fatal error for different types of requests.) Ignoring the implementation for a sec, what behaviour do we want?

It might be reasonable to have two requests queues, one for read requests and one for write requests. That way we can keep seeing updates, even while our chat messages are throttled and being retried. Maybe by default they could limit us to one request per five seconds, or maybe a smaller limit that increases if we keep sending a lot of requests. Or maybe the read queue could allow a couple requests to be in-flight at once, while writing is limited to a single request.

banksJeremy commented 10 years ago

I've been trying some stuff, and I'm leaning towards an approach based on the Executors and Futures in Python 3's concurrent.futures module (using the backported version available on PyPi).

I'm trying to implement a class RequestExecutor(concurrent.futures.Executor). Whenever we want to make a request, we request = executor.submit(fn, *args, **kwargs) the request call, which gives us a RequestFuture(concurrent.futures.Future) instance that will get the request result. If we want to block and wait for the request's result or raised exception, we just need to result = request.result(), or can register a callback.

Internally, the RequestExecutor would have a worker thread which runs a single request at a time. It enforces a minimum interval between consecutive request calls. If a request does raise RequestAttemptFailed(min_interval=0.0), it will be retried, up to a maximum number of times, and after waiting at least min_interval. If the request raises any other type of exception, it will fail immediately and will not be retried. (So if we wanted to retry on HTTP errors, our request methods would need to catch those specific errors that we want and re-raise RequestAttemptFailed. We can probably put some of this boilerplate in a decorator, but we don't want to be reckless and retry more types of exceptions than we should.)

As suggested in the OP, this might be used by having a ._read_request_executor and a ._write_request_executor on Client, so that each class of request does not block the other.

(My implementation is at about 150 lines, but it's broken.)