You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "Roiocam (via GitHub)" <gi...@apache.org> on 2024/02/03 09:11:13 UTC
[PR] additional mailbox selector for typed props [incubator-pekko]
Roiocam opened a new pull request, #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096
## Motivation
I have experience in migrating Classic to Typed. In the process, I found that Typed can't customize Mailbox through Props. We should match the same Classic method on Typed.
<img width="790" alt="截屏2024-02-03 17 03 39" src="https://github.com/apache/incubator-pekko/assets/26020358/dd1bfc1c-4c2b-4926-b54a-3e7129038428">
## TODO list
- [ ] Implementation: Reuse existing classes
- [ ] Mention in documentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479477688
##########
actor-typed-tests/src/test/scala/docs/org/apache/pekko/typed/MailboxDocSpec.scala:
##########
@@ -21,6 +21,7 @@ import pekko.actor.typed.Behavior
import pekko.actor.typed.MailboxSelector
import pekko.actor.typed.scaladsl.Behaviors
import com.typesafe.config.ConfigFactory
+import org.apache.pekko.actor.typed.Dispatchers
Review Comment:
can you remove the org.apache from this import?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479519497
##########
actor-typed/src/main/scala/org/apache/pekko/actor/typed/Props.scala:
##########
@@ -81,6 +81,13 @@ abstract class Props private[pekko] () extends Product with Serializable {
*/
def withDispatcherFromConfig(path: String): Props = DispatcherFromConfig(path, this)
+ /**
+ * Prepend a selection of the mailbox found at the given Config path to this Props.
+ * The path is relative to the configuration root of the [[ActorSystem]] that looks up the
+ * mailbox.
Review Comment:
Make sense, fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479320548
##########
actor-typed/src/main/resources/reference.conf:
##########
@@ -26,7 +26,7 @@ pekko.actor.typed {
# buffer. If the capacity is exceed then additional incoming messages are dropped.
restart-stash-capacity = 1000
- # Typed mailbox defaults to the single consumer mailbox as balancing dispatcher is not supported
+ # Typed mailbox defaults to the single consumber mailbox as balancing dispatcher is not supported
Review Comment:
Thanks, that's previous commit and a rebase operation messed up these file change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1478094046
##########
actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/scaladsl/DispatcherSelectorSpec.scala:
##########
@@ -16,8 +16,10 @@ package org.apache.pekko.actor.typed.scaladsl
import com.typesafe.config.Config
import com.typesafe.config.ConfigFactory
import org.scalatest.wordspec.AnyWordSpecLike
-
import org.apache.pekko
+import org.apache.pekko.actor.typed.DispatcherSelector
Review Comment:
can you remove the org.apache from these new imports?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "nvollmar (via GitHub)" <gi...@apache.org>.
nvollmar commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479303018
##########
actor-typed/src/main/resources/reference.conf:
##########
@@ -26,7 +26,7 @@ pekko.actor.typed {
# buffer. If the capacity is exceed then additional incoming messages are dropped.
restart-stash-capacity = 1000
- # Typed mailbox defaults to the single consumer mailbox as balancing dispatcher is not supported
+ # Typed mailbox defaults to the single consumber mailbox as balancing dispatcher is not supported
Review Comment:
`consumer` was correct
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479508598
##########
actor-typed-tests/src/test/scala/docs/org/apache/pekko/typed/MailboxDocSpec.scala:
##########
@@ -21,6 +21,7 @@ import pekko.actor.typed.Behavior
import pekko.actor.typed.MailboxSelector
import pekko.actor.typed.scaladsl.Behaviors
import com.typesafe.config.ConfigFactory
+import org.apache.pekko.actor.typed.Dispatchers
Review Comment:
Fixed, Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1478234531
##########
actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/scaladsl/DispatcherSelectorSpec.scala:
##########
@@ -95,19 +105,13 @@ class DispatcherSelectorSpec(config: Config)
}
"select same dispatcher as parent, several levels" in {
- val grandParent = spawn(SpawnProtocol(), Props.empty.withDispatcherFromConfig("ping-pong-dispatcher"))
- val parentProbe = createTestProbe[ActorRef[SpawnProtocol.Spawn[Ping]]]()
- grandParent ! SpawnProtocol.Spawn(
- SpawnProtocol(),
- "parent",
- Props.empty.withDispatcherSameAsParent,
- parentProbe.ref)
-
- val childProbe = createTestProbe[ActorRef[Ping]]()
- grandParent ! SpawnProtocol.Spawn(PingPong(), "child", Props.empty.withDispatcherSameAsParent, childProbe.ref)
Review Comment:
Fix children not spawn from the parent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "nvollmar (via GitHub)" <gi...@apache.org>.
nvollmar commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479303993
##########
actor/src/main/resources/reference.conf:
##########
@@ -323,8 +323,8 @@ pekko {
# Probability of doing an exploration v.s. optimization.
chance-of-exploration = 0.4
- # When downsizing after a long streak of under-utilization, the resizer
- # will downsize the pool to the highest utilization multiplied by a
+ # When downsizing after a long streak of underutilization, the resizer
+ # will downsize the pool to the highest utiliziation multiplied by a
Review Comment:
the next line also has an `a`, remove one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "nvollmar (via GitHub)" <gi...@apache.org>.
nvollmar commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479302729
##########
actor-typed/src/main/resources/reference.conf:
##########
@@ -73,7 +73,7 @@ pekko.reliable-delivery {
# To avoid head of line blocking from serialization and transfer
# of large messages this can be enabled.
# Large messages are chunked into pieces of the given size in bytes. The
- # chunked messages are sent separately and assembled on the consumer side.
+ # chunked messages are sent separatetely and assembled on the consumer side.
Review Comment:
`separately` was correct
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479511004
##########
actor-typed/src/main/scala/org/apache/pekko/actor/typed/Props.scala:
##########
@@ -81,6 +81,13 @@ abstract class Props private[pekko] () extends Product with Serializable {
*/
def withDispatcherFromConfig(path: String): Props = DispatcherFromConfig(path, this)
+ /**
+ * Prepend a selection of the mailbox found at the given Config path to this Props.
+ * The path is relative to the configuration root of the [[ActorSystem]] that looks up the
+ * mailbox.
Review Comment:
can you add `@since 1.1.0` on new API methods?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1478231678
##########
actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/scaladsl/DispatcherSelectorSpec.scala:
##########
@@ -16,8 +16,10 @@ package org.apache.pekko.actor.typed.scaladsl
import com.typesafe.config.Config
import com.typesafe.config.ConfigFactory
import org.scalatest.wordspec.AnyWordSpecLike
-
import org.apache.pekko
+import org.apache.pekko.actor.typed.DispatcherSelector
Review Comment:
Thanks, fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#issuecomment-1926744263
@Roiocam can you put the typo fixes in a separate PR? We can backfit that change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #1096:
URL: https://github.com/apache/incubator-pekko/pull/1096#discussion_r1479323075
##########
actor/src/main/resources/reference.conf:
##########
@@ -323,8 +323,8 @@ pekko {
# Probability of doing an exploration v.s. optimization.
chance-of-exploration = 0.4
- # When downsizing after a long streak of under-utilization, the resizer
- # will downsize the pool to the highest utilization multiplied by a
+ # When downsizing after a long streak of underutilization, the resizer
+ # will downsize the pool to the highest utiliziation multiplied by a
Review Comment:
Thanks, this is a new typo found.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] additional mailbox selector for typed props [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1096:
URL: https://github.com/apache/pekko/pull/1096#discussion_r1580657918
##########
actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/scaladsl/DispatcherSelectorSpec.scala:
##########
@@ -16,8 +16,10 @@ package org.apache.pekko.actor.typed.scaladsl
import com.typesafe.config.Config
import com.typesafe.config.ConfigFactory
import org.scalatest.wordspec.AnyWordSpecLike
-
import org.apache.pekko
+import pekko.actor.typed.DispatcherSelector
Review Comment:
can you reorder the imports and put back the empty line above?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org