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

fsockopen UDP connections should be non-blocking #130

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

kiyanwang
Copy link
Member

@kiyanwang kiyanwang commented Oct 8, 2018

We are using fsockopen to create a stream so we can, connect and write to our StatsD server via UDP:

      if (!empty($this->host)) // if host is configured, send..
            {
                $fp = fsockopen("udp://{$this->host}", $this->port);
                if (! $fp) { return; }
                foreach ($sampledData as $stat => $value)
                {
                    if (is_array($value))
                    {
                        foreach ($value as $v)
                        {
                            fwrite($fp, "$stat:$v");
                        }
                    }
                    else
                    {
                        fwrite($fp, "$stat:$value");
                    }
                }
                fclose($fp);
            }
        }

From the PHP manual for fsockopen

The socket will by default be opened in blocking mode. You can switch it to non-blocking mode by using stream_set_blocking().

However, we are not telling PHP to make the stream non-blocking. As a consequence when this attempts to write to the StatsD server if the UDP buffer on the server is full, this call will block on the client and wait.

This P/R sets the stream to non blocking. This means that when it attempts to write, if the server is busy or the UDP buffer on the server is full this will no longer block and instead will return an error which we already ignore.

@kiyanwang kiyanwang changed the title fsockopen UDP connections are not non-blocking fsockopen UDP connections should be non-blocking Oct 8, 2018
@kiyanwang kiyanwang self-assigned this Oct 8, 2018
Copy link

@camillef camillef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good spot!

Copy link
Contributor

@rgubby rgubby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple change but could make a massive difference to overall performance of any app using Tripod!

@kiyanwang
Copy link
Member Author

I've deployed this fix to a tarl on demand staging environment and enabled sending stats to statsd on that box. I also had another ondemand instance that did not contain this fix. I created the same 224 item list (using a ris file) on both servers and proceeded load the list up in shipshape and monitor the response trace details in new relic.

The box with the fix was consistently faster than the box without. This isn't particularly empirical because there's no way these two boxes on their own would generate enough traffic to our statsd server to cause the udp buffer to fill up. What this did demonstrate is that there was no adverse affect after including this change,

@kiyanwang kiyanwang merged commit d7a586d into master Oct 9, 2018
@kiyanwang kiyanwang deleted the non-blocking-statsd branch October 9, 2018 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants