Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HTTP] Don't pipeline non-idempotent method requests #419

Closed
krizhanovsky opened this issue Feb 9, 2016 · 5 comments
Closed

[HTTP] Don't pipeline non-idempotent method requests #419

krizhanovsky opened this issue Feb 9, 2016 · 5 comments
Assignees
Milestone

Comments

@krizhanovsky
Copy link
Contributor

RFC 7230 6.3.2 prohibits pipelining of non-idempotent method requests. So if we receive such client request, then:

  1. we still should read following messages from client connection, process them and enqueue them to outgoing server connections, but don't call ss_send() for them immediately;
  2. if we receive idempotent request from a client, we should check whether there are non-idempotent requests in the server connection waiting to be sent. Probably we should manage this using some connection flag. In this sense non-idempotent request is synchronous "barrier" for requests transmission;
  3. if we receive a response from a server for non-idempotent request, then typically we have some queued idempotent requests, so after the response processing we must call ss_send() for waiting idempotent requests.

See also Making HTTP Pipelining Usable on the Open Web.

@krizhanovsky
Copy link
Contributor Author

Consider this recommendations and the author's blog post about proper implementation of HTTP pipelining.

@krizhanovsky
Copy link
Contributor Author

Actually the definition of idempotent request is application specific(see my presentation, slides 6-8), e.g. GET /update?a=2 could be non-idempotent while POST request with Web-search parameters can be idempotent. So a configuration options like existing cache_methods and caching policy must be introduced to specify idempotent requests. Some DDoS mitigation techniques switches on caching of responses for non-idempotent requests (e.g. we can send the same responses to non-idempotent POSTs), but we still not pipeline them to avoid consistency issues on application side.

keshonok added a commit that referenced this issue Oct 7, 2016
Limit the number of reconnect attempts, and use the initial timeout
between the attempts. The timeout still grows exponentially as the
number of attempts increases.
keshonok added a commit that referenced this issue Oct 7, 2016
If another request comes from a client after an non-idempotent
request, then the client knows what it is doing and permits
pipelining of these requests. In that case, remove the flag of
non-idempotency from the preceding request.
keshonok added a commit that referenced this issue Oct 10, 2016
The "on hold" state is now derived on the fly as it can be dynamic.
If another request comes from a client after an non-idempotent
request, then the client knows what it is doing, and these requests
can be pipelined. In that case remove the flag of non-idempotency
from the preceding request.
keshonok added a commit that referenced this issue Oct 13, 2016
Non-idempotent requests make up an internal @nip_queue within the
server's fwd_queue. @nipcnt keeps the count of those requests in
@nip_queue that can be used by schedulers when making decision.
Special care is taken for the case where a non-idempotent request
is followed by another requests, which re-enables pipelining of
those messages.
keshonok added a commit that referenced this issue Oct 14, 2016
Non-idempotent requests make up an internal @nip_queue within the
server's fwd_queue. @nipcnt keeps the count of those requests in
@nip_queue that can be used by schedulers when making decision.
Special care is taken for the case where a non-idempotent request
is followed by another requests, which re-enables pipelining of
those messages.
keshonok added a commit that referenced this issue Oct 14, 2016
Non-idempotent requests make up an internal @nip_queue within the
server's fwd_queue. @nipcnt keeps the count of those requests in
@nip_queue that can be used by schedulers when making decision.
Special care is taken for the case where a non-idempotent request
is followed by another requests, which re-enables pipelining of
those messages.
keshonok added a commit that referenced this issue Oct 14, 2016
Limit the number of reconnect attempts, and use the initial timeout
between the attempts. The timeout still grows exponentially as the
number of attempts increases.
keshonok added a commit that referenced this issue Oct 14, 2016
The "on hold" state is now derived on the fly as it can be dynamic.
If another request comes from a client after an non-idempotent
request, then the client knows what it is doing, and these requests
can be pipelined. In that case remove the flag of non-idempotency
from the preceding request.
keshonok added a commit that referenced this issue Oct 14, 2016
Non-idempotent requests make up an internal @nip_queue within the
server's fwd_queue. @nipcnt keeps the count of those requests in
@nip_queue that can be used by schedulers when making decision.
Special care is taken for the case where a non-idempotent request
is followed by another requests, which re-enables pipelining of
those messages.
keshonok added a commit that referenced this issue Feb 13, 2017
Non-idempotent requests make up an internal @nip_queue within the
server's fwd_queue. @nipcnt keeps the count of those requests in
@nip_queue that can be used by schedulers when making decision.
Special care is taken for the case where a non-idempotent request
is followed by another requests, which re-enables pipelining of
those messages.
keshonok added a commit that referenced this issue Feb 13, 2017
This commit implements the logic of re-sending requests that were
in the server connection's queue when the connection failed. When
the connection is restored, it's not scheduled until all requests
in the forwarding queue are re-sent or sent to the server.
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
keshonok added a commit that referenced this issue Feb 13, 2017
@keshonok
Copy link
Contributor

keshonok commented Mar 3, 2017

Implemented in #660 (merge commit c40924b).

@keshonok keshonok closed this as completed Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants