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

configuring STREAM_SELECT_USEC #18

Open
rbro opened this issue Sep 10, 2018 · 3 comments
Open

configuring STREAM_SELECT_USEC #18

rbro opened this issue Sep 10, 2018 · 3 comments

Comments

@rbro
Copy link

rbro commented Sep 10, 2018

Hello,

I am making an async request, and while waiting for the response, I want to do other work. In my case, it's calling a check heartbeat function in rabbitmq, so I don't lose the connection while my request is processing - related to what I wrote at the bottom of #6. I am using a pattern like:

while (true)
{
	if ($client->hasResponse($requestId))
	{
		$client->handleResponse($requestId);
		break;
	}

	// do other work here
}

What I noticed is that in hasResponse, Socket::STREAM_SELECT_USEC is 20000 microseconds, which is a little quick for what I need. I would prefer to wait a little longer each iteration as the check heartbeat function doesn't need to run that often.

Would it be possible to allow the user to change STREAM_SELECT_USEC to be another value? Right now it's a constant, so it can't be changed.

Thanks again for your help.

@hollodotme
Copy link
Owner

hollodotme commented Sep 13, 2018

@rbro Thanks for the issue.

First of all: Congratulations, you spotted a bug. 🎉

The value of STREAM_SELECT_USEC should be 200000, not 20000. (See http://php.net/stream_select, red box)

Can you please open another issue for that and provide a simple PR which changes that value?
That would be great and much appreciated!


I quickly looked where I could add a custom value for STREAM_SELECT_USEC best.
Currently I see three options:

Option 1

Configure it via ConfiguresSocketConnection interface (UnixDomainSocket or NetworkSocket) and at the 200000 as a constant to the SocketConnections/Defaults.php.
That means the configured value applies to ALL requests/checks on that connection. I don't know if that is a good idea.

Option 2

Introduce a new optional parameter to the following methods:

  • Client->hasResponse()
  • Client->waitForResponse()
  • Client->waitForResponses()
  • Client->getRequestIdsHavingResponse()
  • Client->readReadyResponses()
  • Client->handleReadyResponses()

This would introduce quite a lot of complexity and harms the readability of the current lib API, IMO.
Even though this is a more flexible approach when you want to have wait-times that differ for each request. And that sentence leads to...

Option 3

Introduce a new getter/setter in the request classes for the wait-time and add it to the internal socket instance, so each socket can have a custom wait-time for stream_select.

But that means that it just applies to Client->hasResponse() and Client->waitForResponse(), because those methods use the stream_select call inside of Socket.php which checks only one socket. All other Client methods above use a stream_select call for multiple sockets in Client-> getRequestIdsHavingResponse() to reduce the amount of stream_select calls. So in most cases (multi-request-checks) the custom value wouldn't be applied.

Long story short:

None of the 3 options is a good / flexible solution in my opinion. Furthermore they add complexity and maybe even a BC break for a use-case which can be solved by one line of user-land code:

<?php

while (true)
{
	if ($client->hasResponse($requestId))
	{
		$client->handleResponse($requestId);
		break;
	}

        usleep(500000); # Wait half a second

	// do other work here
}

If no one else has strong arguments against it or better solutions, I'd close and label this issue as wontfix. 😬

@rbro
Copy link
Author

rbro commented Sep 13, 2018

@hollodotme - thanks, I'll submit a PR soon. I'll think through possible options too, but I'm doing now what you wrote at the end with usleep, but it has a side effect and is why I noticed the issue above. My message queue processing times were a lot slower than what they should have been when processing say 1,000's of messages. I was losing up to half a second for each message I was processing from rabbitmq because if the fastcgi server returned while usleep was running, the script would have to wait for usleep to finish before calling stream_select again. It would be much more efficient to have that waiting be done in the call to stream_select because then the response from the fastcgi server will be processed as soon as it comes in rather than waiting for usleep to finish.

@hollodotme
Copy link
Owner

@rbro Well, that is a valid point. 👍

I am currently in the middle of organizing a PHP conference which happens next weekend, so I have little time to do some prototyping and testing during the next 2 weeks. But I'll surely try to wrap my head around it and find a proper solution. Any input is welcome in the meanwhile. 😄

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