Skip to content

Commit

Permalink
Internal dispatcher to protect against starvation (akka#26816)
Browse files Browse the repository at this point in the history
* Allow for dispatcher aliases and define a internal dispatcher
* Test checking dispatcher name
* MiMa for Dispatchers
* Migration guide entry
* No need to have custom dispatcher lookup logic in streams anymore
* Default dispatcher size and migration note about that
* Test checking exact config values...
* Typed receptionist on internal dispatcher
* All internal usages of system.dispatcher gone through
  • Loading branch information
johanandren authored and patriknw committed May 2, 2019
1 parent e34a711 commit 81b1e2e
Show file tree
Hide file tree
Showing 57 changed files with 524 additions and 329 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
/*
* Copyright (C) 2009-2019 Lightbend Inc. <https://www.lightbend.com>
*/

package akka.actor

import akka.ConfigurationException
import akka.actor.setup.ActorSystemSetup
import akka.dispatch.{ Dispatchers, ExecutionContexts }
import akka.testkit.{ AkkaSpec, ImplicitSender, TestActors, TestProbe }
import com.typesafe.config.ConfigFactory

import scala.concurrent.ExecutionContext
import scala.concurrent.duration._

object ActorSystemDispatchersSpec {

class SnitchingExecutionContext(testActor: ActorRef, underlying: ExecutionContext) extends ExecutionContext {

def execute(runnable: Runnable): Unit = {
testActor ! "called"
underlying.execute(runnable)
}

def reportFailure(t: Throwable): Unit = {
testActor ! "failed"
underlying.reportFailure(t)
}
}

}

class ActorSystemDispatchersSpec extends AkkaSpec(ConfigFactory.parseString("""
dispatcher-loop-1 = "dispatcher-loop-2"
dispatcher-loop-2 = "dispatcher-loop-1"
""")) with ImplicitSender {

import ActorSystemDispatchersSpec._

"The ActorSystem" must {

"work with a passed in ExecutionContext" in {
val ecProbe = TestProbe()
val ec = new SnitchingExecutionContext(ecProbe.ref, ExecutionContexts.global())

val system2 = ActorSystem(name = "ActorSystemDispatchersSpec-passed-in-ec", defaultExecutionContext = Some(ec))

try {
val ref = system2.actorOf(Props(new Actor {
def receive = {
case "ping" => sender() ! "pong"
}
}))

val probe = TestProbe()

ref.tell("ping", probe.ref)

ecProbe.expectMsg(1.second, "called")
probe.expectMsg(1.second, "pong")
} finally {
shutdown(system2)
}
}

"not use passed in ExecutionContext if executor is configured" in {
val ecProbe = TestProbe()
val ec = new SnitchingExecutionContext(ecProbe.ref, ExecutionContexts.global())

val config = ConfigFactory.parseString("akka.actor.default-dispatcher.executor = \"fork-join-executor\"")
val system2 = ActorSystem(
name = "ActorSystemDispatchersSpec-ec-configured",
config = Some(config),
defaultExecutionContext = Some(ec))

try {
val ref = system2.actorOf(TestActors.echoActorProps)
val probe = TestProbe()

ref.tell("ping", probe.ref)

ecProbe.expectNoMessage(200.millis)
probe.expectMsg(1.second, "ping")
} finally {
shutdown(system2)
}
}

def userGuardianDispatcher(system: ActorSystem): String = {
val impl = system.asInstanceOf[ActorSystemImpl]
impl.guardian.asInstanceOf[ActorRefWithCell].underlying.asInstanceOf[ActorCell].dispatcher.id
}

"provide a single place to override the internal dispatcher" in {
val sys = ActorSystem(
"ActorSystemDispatchersSpec-override-internal-disp",
ConfigFactory.parseString("""
akka.actor.internal-dispatcher = akka.actor.default-dispatcher
"""))
try {
// that the user guardian runs on the overriden dispatcher instead of internal
// isn't really a guarantee any internal actor has been made running on the right one
// but it's better than no test coverage at all
userGuardianDispatcher(sys) should ===("akka.actor.default-dispatcher")
} finally {
shutdown(sys)
}
}

"provide internal execution context instance through BootstrapSetup" in {
val ecProbe = TestProbe()
val ec = new SnitchingExecutionContext(ecProbe.ref, ExecutionContexts.global())

// using the default for internal dispatcher and passing a pre-existing execution context
val system2 =
ActorSystem(
name = "ActorSystemDispatchersSpec-passed-in-ec-for-internal",
config = Some(ConfigFactory.parseString("""
akka.actor.internal-dispatcher = akka.actor.default-dispatcher
""")),
defaultExecutionContext = Some(ec))

try {
val ref = system2.actorOf(Props(new Actor {
def receive = {
case "ping" => sender() ! "pong"
}
}).withDispatcher(Dispatchers.InternalDispatcherId))

val probe = TestProbe()

ref.tell("ping", probe.ref)

ecProbe.expectMsg(1.second, "called")
probe.expectMsg(1.second, "pong")
} finally {
shutdown(system2)
}
}

"use an internal dispatcher for the guardian by default" in {
userGuardianDispatcher(system) should ===("akka.actor.internal-dispatcher")
}

"use the default dispatcher by a user provided user guardian" in {
val sys = new ActorSystemImpl(
"ActorSystemDispatchersSpec-custom-user-guardian",
ConfigFactory.defaultReference(),
getClass.getClassLoader,
None,
Some(Props.empty),
ActorSystemSetup.empty)
sys.start()
try {
userGuardianDispatcher(sys) should ===("akka.actor.default-dispatcher")
} finally shutdown(sys)
}

"provide a good error on an dispatcher alias loop in the config" in {
intercept[ConfigurationException] {
system.dispatchers.lookup("dispatcher-loop-1")
}
}

}

}
62 changes: 2 additions & 60 deletions akka-actor-tests/src/test/scala/akka/actor/ActorSystemSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ import akka.actor.setup.ActorSystemSetup
import akka.dispatch._
import akka.japi.Util.immutableSeq
import akka.pattern.ask
import akka.testkit._
import akka.testkit.TestKit
import akka.testkit.{ TestKit, _ }
import akka.util.Helpers.ConfigOps
import akka.util.{ Switch, Timeout }
import com.github.ghik.silencer.silent
import com.typesafe.config.{ Config, ConfigFactory }

import scala.concurrent.duration._
import scala.concurrent.{ Await, ExecutionContext, Future }
import scala.concurrent.{ Await, Future }
import scala.language.postfixOps
import scala.util.Properties

Expand Down Expand Up @@ -107,19 +106,6 @@ object ActorSystemSpec {
override def dispatcher(): MessageDispatcher = instance
}

class TestExecutionContext(testActor: ActorRef, underlying: ExecutionContext) extends ExecutionContext {

def execute(runnable: Runnable): Unit = {
testActor ! "called"
underlying.execute(runnable)
}

def reportFailure(t: Throwable): Unit = {
testActor ! "failed"
underlying.reportFailure(t)
}
}

val config = s"""
slow {
type="${classOf[SlowDispatcher].getName}"
Expand Down Expand Up @@ -372,50 +358,6 @@ class ActorSystemSpec extends AkkaSpec(ActorSystemSpec.config) with ImplicitSend
}
}

"work with a passed in ExecutionContext" in {
val ecProbe = TestProbe()
val ec = new ActorSystemSpec.TestExecutionContext(ecProbe.ref, ExecutionContexts.global())

val system2 = ActorSystem(name = "default", defaultExecutionContext = Some(ec))

try {
val ref = system2.actorOf(Props(new Actor {
def receive = {
case "ping" => sender() ! "pong"
}
}))

val probe = TestProbe()

ref.tell("ping", probe.ref)

ecProbe.expectMsg(1.second, "called")
probe.expectMsg(1.second, "pong")
} finally {
shutdown(system2)
}
}

"not use passed in ExecutionContext if executor is configured" in {
val ecProbe = TestProbe()
val ec = new ActorSystemSpec.TestExecutionContext(ecProbe.ref, ExecutionContexts.global())

val config = ConfigFactory.parseString("akka.actor.default-dispatcher.executor = \"fork-join-executor\"")
val system2 = ActorSystem(name = "default", config = Some(config), defaultExecutionContext = Some(ec))

try {
val ref = system2.actorOf(TestActors.echoActorProps)
val probe = TestProbe()

ref.tell("ping", probe.ref)

ecProbe.expectNoMessage(200.millis)
probe.expectMsg(1.second, "ping")
} finally {
shutdown(system2)
}
}

"not allow top-level actor creation with custom guardian" in {
val sys = new ActorSystemImpl(
"custom",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class ConfigSpec extends AkkaSpec(ConfigFactory.defaultReference(ActorSystem.fin
{
val pool = c.getConfig("fork-join-executor")
pool.getInt("parallelism-min") should ===(8)
pool.getDouble("parallelism-factor") should ===(3.0)
pool.getDouble("parallelism-factor") should ===(1.0)
pool.getInt("parallelism-max") should ===(64)
pool.getString("task-peeking-mode") should be("FIFO")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ class DispatcherShutdownSpec extends WordSpec with Matchers {
.dumpAllThreads(false, false)
.toList
.map(_.getThreadName)
.filter(_.startsWith("DispatcherShutdownSpec-akka.actor.default"))
.filter(name =>
name.startsWith("DispatcherShutdownSpec-akka.actor.default") || name.startsWith(
"DispatcherShutdownSpec-akka.actor.internal")) // nothing is run on default without any user actors started
.size

val system = ActorSystem("DispatcherShutdownSpec")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

package akka.actor.typed

import akka.annotation.InternalApi

import scala.concurrent.ExecutionContextExecutor

object Dispatchers {
Expand All @@ -13,6 +15,11 @@ object Dispatchers {
* configuration of the default dispatcher.
*/
final val DefaultDispatcherId = "akka.actor.default-dispatcher"

/**
* INTERNAL API
*/
@InternalApi final val InternalDispatcherId = "akka.actor.internal-dispatcher"
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@

package akka.actor.typed.receptionist

import akka.actor.typed.{ ActorRef, ActorSystem, Extension, ExtensionId }
import akka.actor.typed.{ ActorRef, ActorSystem, Dispatchers, Extension, ExtensionId, ExtensionSetup, Props }
import akka.actor.typed.internal.receptionist._
import akka.annotation.DoNotInherit

import scala.collection.JavaConverters._
import scala.reflect.ClassTag

import akka.actor.typed.ExtensionSetup
import akka.actor.typed.Props
import akka.annotation.InternalApi

/**
Expand Down Expand Up @@ -51,7 +49,10 @@ abstract class Receptionist extends Extension {
} else LocalReceptionist

import akka.actor.typed.scaladsl.adapter._
system.internalSystemActorOf(provider.behavior, "receptionist", Props.empty)
system.internalSystemActorOf(
provider.behavior,
"receptionist",
Props.empty.withDispatcherFromConfig(Dispatchers.InternalDispatcherId))
}
}

Expand Down
5 changes: 5 additions & 0 deletions akka-actor/src/main/mima-filters/2.5.x.backwards.excludes
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# excludes for 2.6

ProblemFilters.exclude[MissingClassProblem]("akka.actor.Inbox$")
ProblemFilters.exclude[MissingClassProblem]("akka.actor.Inbox")
ProblemFilters.exclude[MissingClassProblem]("akka.actor.ActorDSL$")
Expand All @@ -13,3 +15,6 @@ ProblemFilters.exclude[DirectMissingMethodProblem]("akka.actor.ActorRefFactory.a
ProblemFilters.exclude[DirectMissingMethodProblem]("akka.actor.ActorSystem.actorFor")
ProblemFilters.exclude[DirectMissingMethodProblem]("akka.actor.ChildActorPath.this")
ProblemFilters.exclude[MissingClassProblem]("akka.actor.dungeon.UndefinedUidActorRef")

# Protect internals against starvation #23576
ProblemFilters.exclude[DirectMissingMethodProblem]("akka.dispatch.Dispatchers.this")
Loading

0 comments on commit 81b1e2e

Please sign in to comment.