Page MenuHomePhabricator

In taskmaster daemons, only close connections which were not used recently
ClosedPublic

Authored by epriestley on Aug 11 2016, 4:07 PM.
Tags
None
Referenced Files
F13434828: D16389.id.diff
Thu, Jul 11, 12:51 AM
F13433219: D16389.diff
Wed, Jul 10, 2:30 PM
F13424096: D16389.diff
Tue, Jul 9, 2:15 AM
F13415408: D16389.id39413.diff
Sun, Jul 7, 7:30 PM
F13410134: D16389.id39416.diff
Sat, Jul 6, 2:01 PM
F13404741: D16389.diff
Fri, Jul 5, 1:56 PM
F13402938: D16389.id.diff
Thu, Jul 4, 7:26 PM
F13394132: D16389.id.diff
Wed, Jul 3, 5:32 AM
Subscribers
None

Details

Summary

Ref T11458. Depends on D16388. Currently, we're very aggressive about closing connections in the taskmaster daemons.

This can end up taking up a lot of resources. In particular, because the outgoing port for outbound connections normally can not be reused for 60 seconds after a connection closes, we may exhaust outbound ports on the host if there's a big queue full of stuff that's being processed very quickly.

At a minimum, we always are holding open a worker connection, which we always need again right away. So even in the best case we end up opening/closing this about once per second and each daemon takes up about ~60 outbound ports when it should take up ~1.

So, make two adjustments:

  • First, only close connections which we haven't issued a query on in the last 60 seconds. This should prevent us from closing connections that we'll need again immediately in most cases. In the worst case, we shouldn't be eating up any extra ports under default TCP behavior.
  • Second, explicitly close connections. We were relying on implicit/GC behavior (maybe as a holdover from very long ago, before we got connection wrappers in place?), which probably did about the same thing but isn't as predictable and can't be profiled or instrumented.
Test Plan

This is somewhat difficult to test completely convincingly in isolation since the problem behavior depends on production scales and the workload, and to some degree on configuration.

I tested that this stuff baiscally works by adding logging to connect/close and running the daemons, verifying that they churned connections a lot before this change (e.g., ~1/s even at no load) and churn rarely afterward (e.g., almost never at no load).

I ran some workload through them to make sure I didn't completely break anything.

The best real test is just seeing how production responds. Current inbound/outbound connections on secure001 are 1,200:

secure001 $ netstat -t | grep :mysql | wc -l
1164

Current outbound from repo001 are 18,600:

repo001 $ netstat -t | grep :mysql | wc -l
18663

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to In taskmaster daemons, only close connections which were not used recently.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
  • Actually close after 60 seconds inactive instead of 15 (which I was testing with), to align with default TCP behavior.
chad edited edge metadata.
This revision is now accepted and ready to land.Aug 11 2016, 6:19 PM
This revision was automatically updated to reflect the committed changes.