Skip to content

Comments

fix: typed actor dead letter address (#29795)#32104

Merged
johanandren merged 6 commits intoakka:mainfrom
Roiocam:roiocam-dead-letters-fix
Sep 18, 2023
Merged

fix: typed actor dead letter address (#29795)#32104
johanandren merged 6 commits intoakka:mainfrom
Roiocam:roiocam-dead-letters-fix

Conversation

@Roiocam
Copy link
Contributor

@Roiocam Roiocam commented Sep 16, 2023

Motivation

Attach the necessary information before sending the message to DeadLetter to correctly display the destination in the DeadLetter.

References #29795

How does this PR fix work?

In classic Actors, when an Actor dies, its Mailbox is "swap" to a deadLetterMailbox Mailbox.

截屏2023-09-17 05 18 26

When an Actor sends a message to another ActorRef, it essentially enqueues the message to that Actor's mailbox.

When an Actor sends a message to a dead Actor, it means that the message would enqueue to the "deadLetterMailbox",the dead mailbox will transform the message into a DeadLetter event and then publish it to the EventBus. This is how DeadLetters work in classic Actors.

In Typed actor, it basiclly same, but the sender variable cannot be passed implicitly.

In the Ask pattern, there are some differences. When an Actor uses ask to request another Actor, it essentially creates a temporary PromiseActorRef to receive the response from the target Actor.

When the PromiseActorRef receives a tell call, it checks if the current Future has already completed and, if so, emits a DeadLetter event. This is done to ensure consistent behavior with the tell method (i.e., sending DeadLetter events to the EventBus via DeadLetterActorRef).

However, the PromiseActorRef directly sends the message without transforming it into a DeadLetter event, resulting in distorted logs.

This PR converts the message into a DeadLetter event before sending it from PromiseActorRef to DeadLetterActorRef.

@Roiocam Roiocam marked this pull request as draft September 16, 2023 21:35

val deadLettersRef = system.classicSystem.deadLetters
deadLetter.recipient shouldNot equal(deadLettersRef)
deadLetter.recipient should equal(ActorRefAdapter.toClassic(actor))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't use actor.toClassic, There should be a better way to optimize this line.

@Roiocam Roiocam marked this pull request as ready for review September 17, 2023 15:35
Copy link
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

val answer: Future[String] = actor.ask(Foo("bar", _))
val result = answer.failed.futureValue
result shouldBe a[TimeoutException]
result.getMessage should startWith("Ask timed out on")
Copy link
Contributor

@leviramsey leviramsey Sep 20, 2023

Choose a reason for hiding this comment

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

This test seems flaky... there's a race between news of the actor stopping and the ask. If one side wins, the message will start with this, but if the other wins, it will start with "Recipient... had already been terminated."

Copy link
Contributor

Choose a reason for hiding this comment

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

#32111 addresses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, It is also has a fix commit on #32107

@Roiocam Roiocam deleted the roiocam-dead-letters-fix branch October 10, 2023 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants