Speed up v2/pods/::status (#6786)
#6791
Conversation
Summary: This replace the Akka stream with a simple `Future.sequence` and uses the same `AppInfoBaseData` instance for each pod. This we utilize the cached instances and not limit ourselves to eight parallel threads. To not overload Marathon we use the fixed size thread pool from #5973. However, even with #5973 we might allocate a lot of futures and runnables. For 100 pods with 1 instance each I got the following: ``` › wrk -c 10 -t 10 -d 2m http://localhost:8080/v2/pods/::status Running 2m test @ http://localhost:8080/v2/pods/::status 10 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 136.52ms 44.93ms 859.05ms 93.64% Req/Sec 7.71 2.49 20.00 69.81% 8918 requests in 2.00m, 1.34GB read Requests/sec: 74.26 Transfer/sec: 11.45MB ``` ``` Running 2m test @ http://localhost:8080/v2/pods/::status 10 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 62.27ms 23.24ms 220.39ms 70.17% Req/Sec 15.94 6.09 50.00 69.12% 19345 requests in 2.00m, 2.91GB read Requests/sec: 161.10 Transfer/sec: 24.85MB ``` JIRA issues: MARATHON-8563
There was a problem hiding this comment.
I'm building your change at jenkins-marathon-pipelines-PR-6791-1.
There was a problem hiding this comment.
✔ Build of #6791 completed successfully.
See details at jenkins-marathon-pipelines-PR-6791-1.
You can create a DC/OS with your patched Marathon by creating a new pull
request with the following changes in buildinfo.json:
"url": "https://s3.amazonaws.com/downloads.mesosphere.io/marathon/builds/1.6.571-b5c64627b/marathon-1.6.571-b5c64627b.tgz",
"sha1": "c7352acef051bcf4b7fe5e56ff90bdc420c8a604"You can run system integration test changes of this PR against Marathon
master by triggering this Jenkins job with the Pull_Request_id 6791.
The job then reports back to this PR.
＼\ ٩( ᐛ )و /／
| @@ -31,6 +31,13 @@ private[appinfo] class DefaultInfoService( | |||
| } | |||
| } | |||
|
|
|||
| override def selectPodStatuses(ids: Set[PathId], selector: PodSelector): Future[Seq[PodStatus]] = { | |||
There was a problem hiding this comment.
I missed the opportunity to review initially, but I'd wondered why this needs to a Set, and then down below, I'd do ids.iterator followed by a to[Seq] to reduce the number of copies etc. Probably trivial optimization stuff :)
There was a problem hiding this comment.
The ids come as a set. I stumbled upon this as well.
Summary:
This replace the Akka stream with a simple
Future.sequenceand usesthe same
AppInfoBaseDatainstance for each pod. This we utilize thecached instances and not limit ourselves to eight parallel threads. To
not overload Marathon we use the fixed size thread pool from #5973.
However, even with #5973 we might allocate a lot of futures and
runnables.
For 100 pods with 1 instance each I got the following:
JIRA issues: MARATHON-8563