Determine if no job servers were connected to

[jmathai]

I didn't see an easy way to determine if connecting to all job servers failed. There's a protected $conn array which could be checked for length but I wasn't sure if there was a better way.

If not I'm happy to submit a pull request to add a method to get a count of servers that have been connected to.

[jmathai]

Nevermind. getConnection without any parameters returns null if no connections were made.

[jmathai]

Damnit, I'm an idiot. It's protected. So back to my initial question.

[till]

You can't use isConnected()? Maybe paste some code to give you more pointers.

In general, we employ a circuit breaker pattern:

• async check the availability (e.g. are our gearmands up, how busy are they)
• use this data to connect to gearmand

[jmathai]

@till You have to give isConnected a connection. I was more interested in handling the case when you create the client but none of the servers were connected to. Could do something similar to what you suggested but thought a simple check on the $conn array might be more straightforward.

[till]

You could extend the Net_Gearman_Client and add a method.

Just looking at the code, I'm not a 100% sure if I like what that class is doing.

• Too much work in __construct().
• You need a custom error handler to get whatever trigger_error() bubbles up.
• No way to figure out easily if anything worked at all.

@brianlmoon Thoughts?

[j03k64]

@jmathai, @till sorry for the delayed response, I just started watching this repo so I wasn't getting notifications before.

Can you give me some psuedo code on how you would expect this to work? Might help me understand better. The constructor throws an Exception that should be catchable, but it would make more sense if it were a more explicit exception like Net_Gearman_NoAvailableServersException instead of the generic Net_Gearman_Exception.

@till I agree with doing too much in constructor. Maybe we should break that out to a separate method.

What are your guys' thoughts? I'd be interested in seeing how you guys would actually use it.

[jmathai]

The Net_Gearman_NoAvailableServersException exception would be sufficient for me. I'm a proponent of keeping this library simple so anything beyond that (i.e. N retries on failure) seems out of context for this library.

This is essentially what I'm doing by checking the length of $conn but that's really a hack and an exception when a connection isn't available would be much better.

[till]

Two years later I have no thoughts on this. Little time right now and internally we moved on pecl gearman. I'll review PRs though and give feedback. :)

[j03k64]

@till from your experience do more peeps use the pecl extension vs. the pear one? I use the @brianlmoon net_gearman lib which I have contributed to, but it seems like more peeps use the pecl extension in my short experience with GearmanManager.

[brianlmoon]

Most of the early examples used pecl so people go to it first. Plus there are docs on php.net for pecl gearman. I find it to be not very controllable and early in GearmanManager development, many people had stability problems with it. For example, setTimeout only takes an int. We have timeout requirements of 200ms. Having a timeout of 1 second or more is useless to me. I need to return pages in 300ms. Our mean 90th connect time for gearmand is around 10ms. So having a connect timeout of 1 second really makes no sense.

[till]

@brianlmoon I bet @hjr3 would address that though.

[till]

@jpirkey I see the benefit in both.

[hjr3]

The http://us2.php.net/manual/en/gearmanclient.settimeout.php clearly states that the integer should be represented in milliseconds. The same for http://us2.php.net/manual/en/gearmanworker.settimeout.php. I thought maybe there was some conversion being done within pecl/gearman, but looking at https://github.com/hjr3/pecl-gearman/blob/master/php_gearman.c#L1878-1890 it is just a pass through. The http://gearman.info/libgearman/gearman_client_st.html?highlight=timeout#gearman_client_set_timeout documentation seems to match up.

Please help me understand what I am missing and I will be happy to make timeout work as you expect.

[till]

Yay! No problem! ;) Thanks, @hjr3 !

[brianlmoon]

I apologize. I read the docs wrong. That is my bad.