Stashing - problems with storing similar messages.

[Horusiath]

Some time ago we seemed to fix problem with error "Cannot stash the same message twice" ocurring when we appended two messages having custom equality by changing it on reference equality instead. This however doesn't solve the problem, as sometimes we may want to stash the "referentialy" the same message twice, like in case of singleton messages or strings.

[Aaronontheweb]

Hmmmm.... Does the Stash store the Envelope itself?

[johlrich]

Stash does store Envelope, yes. I did some digging and this is what I came up with:

I think the problem here is with Akka the actorCell.currentMessage mentioned in stash() is already an Envelope. So it's checking reference equality against Envelopes, not message contents (essentially preventing a user from calling stash more than once in the same receive and saving the same Envelope twice).

CurrentMessage seems to contain the unwrapped envelope's contents in Akka.NET though, causing the observed behavior of preventing the same instance of a message versus the same instance of an Envelope

Not sure what the best next steps would be though

[rogeralsing]

@johlrich is correct. we should refactor the core to keep the current envelope. it should be pretty straight forward to do this.

[edit] btw.
but just to spice things up, our envelope is a struct for predictable GC, so doing reference equality on it will be problematic.
And converting it to a class will have some perf impact as it increased GC pressure, as it will result in one new object to collect per sent message.
Not sure how much this affects a real world system but if I recall correctly, this was visible in the old ping pong benchmark in the early days, thus the struct.

It could be a complete non issue and just me rambling..

[rogeralsing]

I've verified this, there is a 20%+ perf loss in the delivery pipeline if we change Envelope from struct to class

[rogeralsing]

Stupid question from me maybe but do we need to do any sort of comparison at all?
cant we just have a bool CurrentMessageIsStashed in actor cell which starts out as false, and once you call Stash then you can not call stash again for that message.
We only need to guarantee we are not stashing the same message multiple times, right?

[johlrich]

@rogeralsing Hmm. When things like HandleFailed and HandleCompelteTask are executed and replace the CurrentMessage, does that happen in a way that would prevent stashing a legitimate message in the same cycle?

BTW, How was the 20% loss mentioned before measured?

[rogeralsing]

@johlrich HandleFailed is called when a parent actor is supervising a child, all system messages are processed synchronously just like every other message, so when HandleFailed is invoked, there is no other message processing going on in the parent actor at that time.

HandleCompleteTask should not be a problem, as the async await support prevents the actor from processing any other user messages until the async operation completes.

That being said, it does raise a fair question here as the async await support does process system messages while the async user operation is going on.
which could lead to a failing child raising a Failed system event in the parent and if the parent is at the same time processing an async await based receive handler, then things could get messed up.
that is, if both of the above cases occur at the same time.

The best bet would probably be to just prevent actors from processing system messages also during an async receive handler as that would mimic the behavior and semantics from normal synchronous receive handlers.

BTW, How was the 20% loss mentioned before measured?

I didnt quite get what you ment here?

[johlrich]

@rogeralsing I was wondering what you used to measure the performance loss from the struct to class change you mentioned previously. Was this done through the PingPong benchmark or maybe some other measurement?

[Horusiath]

I've marked this problem as critical since it's very serious issue in case of Akka.FSharp and Akka.Persistence.

The problem is not equality of envelopes themselfs - we need a way to recognize if the message has not been stashed twice during the same Receive scope. This is the problem, we need to solve.

[Horusiath]

Fixed by #1480