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

Timeout for incoming requests in io queue. #673

Open
shaitan opened this issue Nov 26, 2015 · 4 comments
Open

Timeout for incoming requests in io queue. #673

shaitan opened this issue Nov 26, 2015 · 4 comments

Comments

@shaitan
Copy link
Member

shaitan commented Nov 26, 2015

Problem

Currently, after whole request is read from client's socket it is put to io queue where it will wait until some io thread will pop it. The time that request is in io queue is not limited, so server node can start handling request when the client is desperate to wait a response, so it has already handled timeout error and probably resent the request or gone. But server node will spend time and resources on handling request and sending response. Situation becomes worse when server node is overloaded: it handles timeouted requests and delays handling still alive requests which will be timeouted when server node will reach them.

What can we do

We can introduce timeout that will limit:

  • time it takes to read the requests from socket - it very depends on a client
  • time that the request spent in the queue - it completely depends on the current server node load

In the first version timeout can be set via server node config, but a client should be able to disable this timeout via command flags.

If that will not be enough, in the second version we can allow a client to redefine this timeout with request, so request will contain field that will be used as timeout for this request.

@bioothod
Copy link
Member

Sending timeout with request could fix this issue, but do we really have an empty data in the header for this?

If server doesn't know client timeout it is dangerous to rely on what server has in its config. It might be the case that client deliberately set really long timeout and he does want to get its data, while server may have just a tiny timeout which will destroy request even without considering doing it.

@toshic
Copy link
Member

toshic commented Nov 27, 2015

At least we have some space in dnet_io_attr header for IO transactions.

@shaitan
Copy link
Member Author

shaitan commented Nov 27, 2015

I know that only server's timeout from config does not cover all cases, but it can be made as a first step without breaking anything and it covers really huge part of cases. Clients that want to get their data in any time can use command flag that will disable server's timeout.

After the first step we will find (or not) clients and cases in which it is better to set timeout by a client. As the next step, if there will be such clients, we can use dnet_io_attr::reserved2 and allow clients to set timeout only for io commands.

And finally, we can break our protocol and add timeout to dnet_cmd.

@shaitan
Copy link
Member Author

shaitan commented Dec 2, 2015

I'll implement first step next week and send a PR with changes. Those who interested in such deadline will can build from the PR and check coverage of their cases. In the PR I will collect feedback of usage and uncovered cases. If there will be a reason I will update the PR to the second step and continue collecting feedback.

It is important that both version will not break common use-case of those who not interested in deadline.

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

3 participants