Added support for `UnrestrictedStash` #6325

ismaelhamed · 2023-01-04T14:50:11Z

Currently, once you've marked your actor with any of the Stash interfaces (IWithUnboundedStash or IWithBoundedStash) there's no way to change it afterward via configuration (it is a design-time operation that requires recompiling).

But there are two traits that allow users to do that in the JVM:

A default Stash trait (IWithStash in this PR) which allows using any Deque-based mailbox (i.e., bounded or unbounded) via configuration. By default, it uses an UnboundedDeque mailbox, so it is akin to the current IWithUnboundedStash if you don't specify a custom mailbox type.
UnrestrictedStash trait, which does not enforce any mailbox type. The proper mailbox must be configured
manually.

Checklist

This change follows the Akka.NET API Compatibility Guidelines.
I have reviewed my own pull request.
Changes in public API reviewed, if any.
I have added website documentation for this feature.

ismaelhamed · 2023-01-04T14:53:34Z

src/core/Akka/Actor/Stash/IWithBoundedStash.cs

-    [Obsolete("Bounded stashing is not yet implemented. Unbounded stashing will be used instead [0.7.0]")]
-    public interface IWithBoundedStash : IActorStash, IRequiresMessageQueue<IBoundedDequeBasedMessageQueueSemantics>
+    [Obsolete("Use `IWithStash` with a configured BoundedDeque-based mailbox instead.")]
+    public interface IWithBoundedStash : IWithUnrestrictedStash, IRequiresMessageQueue<IBoundedDequeBasedMessageQueueSemantics>


We can remove this type since typically you'd want to change the mailbox or stash capacity with a bounded deque, which cannot be done programmatically (AFAIK)

Let's remove this type in v1.5 in a separate PR - I'd like to backport this as-is to v1.4 first.

ismaelhamed · 2023-01-04T14:55:00Z

src/core/Akka/Actor/Stash/StashFactory.cs

-            }
+
+            if (actorType.Implements<IWithUnrestrictedStash>())
+                return new UnrestrictedStashImpl(context);

            throw new ArgumentException($"Actor {actorType} implements an unrecognized subclass of {typeof(IActorStash)} - cannot instantiate", nameof(actorType));
        }


BoundedStashImpl, UnboundedStashImpl and UnrestrictedStashImpl are all empty implementations, so we could get away with a single type (i.e., StashSupportImpl) like in the JVM.

Aaronontheweb · 2023-01-04T16:36:09Z

UnrestrictedStash trait, which does not enforce any mailbox type. The proper mailbox must be configured
manually.

So in practice, how does this work? The user has to manually specify a mailbox type in addition to specifying the stash type?

ismaelhamed · 2023-01-05T06:53:56Z

UnrestrictedStash trait, which does not enforce any mailbox type. The proper mailbox must be configured
manually.

So in practice, how does this work? The user has to manually specify a mailbox type in addition to specifying the stash type?

Correct. By using IWithUnrestrictedStash you're opting out from the framework enforcing any mailbox type, but you must specify one (there's no default mailbox in this case).

ismaelhamed · 2023-01-05T08:13:18Z

Updated the documentation.

As an addendum to my previous comment, you now have two main stash interfaces to work with:

IWithStash, which is "unrestricted" since it allows both bounded and unbounded deque-based mailboxes. Defaults to unbounded if not specified otherwise.
IWithUnboundedStash, if you want to enforce that your actor can only work with an unbounded stash.

The UnrestrictedStash type is there to solve the single inheritance problem in JAVA, which I've respected for the sake of maintaining a compatible API. But since a Stash's mailbox MUST be deque-based anyway, using IWithStash or IWithUnrestrictedStash is basically the same (except that in the latter you must always specify a mailbox).

ismaelhamed · 2023-01-05T10:34:45Z

@Aaronontheweb could we backport this and #6323 to v1.4?

ismaelhamed · 2023-01-18T14:03:03Z

Unrelated tests failing

Aaronontheweb · 2023-01-18T15:54:59Z

I put it on my planner to review this today - will do @ismaelhamed

Aaronontheweb · 2023-01-18T16:37:09Z

@Aaronontheweb could we backport this and #6323 to v1.4?

yep, I don't see why not provided that there's nothing high risk in the PR

Aaronontheweb

LGTM

Aaronontheweb · 2023-01-18T20:32:39Z

docs/articles/actors/receive-actor-api.md


-Here is an example of the `IWithUnboundedStash` interface in action:
+> [!NOTE]
+> The interface `IWithStash` implements the marker interface `IRequiresMessageQueue<IDequeBasedMessageQueueSemantics>` which requests the system to automatically choose a deque-based mailbox implementation for the actor (defaults to an unbounded deque mailbox). If you want more control over the mailbox, see the documentation on mailboxes: [Mailboxes](xref:mailboxes).


Aaronontheweb · 2023-01-18T20:33:09Z

docs/articles/actors/receive-actor-api.md

+The result of this is that when an actor is restarted, any stashed messages will be delivered to the new incarnation of the actor. This is usually the desired behavior.
+
+> [!NOTE]
+> If you want to enforce that your actor can only work with an unbounded stash, then you should use the `IWithUnboundedStash` interface instead.


Aaronontheweb · 2023-01-18T20:33:59Z

src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.Core.verified.txt

    public interface IWithTimers
    {
        Akka.Actor.ITimerScheduler Timers { get; set; }
    }
-    public interface IWithUnboundedStash : Akka.Actor.IActorStash, Akka.Dispatch.IRequiresMessageQueue<Akka.Dispatch.IUnboundedDequeBasedMessageQueueSemantics> { }
+    public interface IWithUnboundedStash : Akka.Actor.IActorStash, Akka.Actor.IWithUnrestrictedStash, Akka.Dispatch.IRequiresMessageQueue<Akka.Dispatch.IUnboundedDequeBasedMessageQueueSemantics> { }


This is extend-only, so it's safe.

Aaronontheweb · 2023-01-18T20:35:27Z

src/core/Akka.API.Tests/verify/CoreAPISpec.ApprovePersistenceSqlCommon.Net.verified.txt

@@ -272,7 +272,7 @@ namespace Akka.Persistence.Sql.Common.Journal
        public long Offset { get; }
        public Akka.Actor.IActorRef ReplyTo { get; }
    }
-    public abstract class SqlJournal : Akka.Persistence.Journal.AsyncWriteJournal, Akka.Actor.IActorStash, Akka.Actor.IWithUnboundedStash, Akka.Dispatch.IRequiresMessageQueue<Akka.Dispatch.IUnboundedDequeBasedMessageQueueSemantics>
+    public abstract class SqlJournal : Akka.Persistence.Journal.AsyncWriteJournal, Akka.Actor.IActorStash, Akka.Actor.IWithUnboundedStash, Akka.Actor.IWithUnrestrictedStash, Akka.Dispatch.IRequiresMessageQueue<Akka.Dispatch.IUnboundedDequeBasedMessageQueueSemantics>


This makes me think that we might have to update all of the Akka.Persistence plugins, since the journal currently uses an unbounded stash. I don't think that should be important unless there's something on the normal IWithUnrestrictedStash interface that could cause a MissingMethodException but it looks to me like there isn't any. Just wanted to make note of this in case I'm wrong.

Not really because the journal is still implementing IWithUnboundedStash which is more constrained. So, the mailbox is going to be selected based on it and not IWithUnrestrictedStash.

Aaronontheweb · 2023-01-18T20:35:55Z

src/core/Akka/Actor/Stash/IWithBoundedStash.cs

-    [Obsolete("Bounded stashing is not yet implemented. Unbounded stashing will be used instead [0.7.0]")]
-    public interface IWithBoundedStash : IActorStash, IRequiresMessageQueue<IBoundedDequeBasedMessageQueueSemantics>
+    [Obsolete("Use `IWithStash` with a configured BoundedDeque-based mailbox instead.")]
+    public interface IWithBoundedStash : IWithUnrestrictedStash, IRequiresMessageQueue<IBoundedDequeBasedMessageQueueSemantics>


Let's remove this type in v1.5 in a separate PR - I'd like to backport this as-is to v1.4 first.

) * Read stash capacity from actor's mailbox or dispatcher configuration (#6323) * Added support for `UnrestrictedStash` (#6325) * Add API for UntypedActorWithStash types (#6327) Co-authored-by: Aaron Stannard <aaron@petabridge.com> Co-authored-by: Aaron Stannard <aaron@petabridge.com>

ismaelhamed commented Jan 4, 2023

View reviewed changes

ismaelhamed force-pushed the unrestricted-stash branch 2 times, most recently from 971318e to 57d9f60 Compare January 5, 2023 08:00

ismaelhamed mentioned this pull request Jan 5, 2023

Add API for UntypedActorWithStash types #6327

Merged

4 tasks

ismaelhamed force-pushed the unrestricted-stash branch from 57d9f60 to bd276df Compare January 5, 2023 09:10

ismaelhamed force-pushed the unrestricted-stash branch 5 times, most recently from 6ddfbf0 to 486c5f6 Compare January 11, 2023 16:53

ismaelhamed force-pushed the unrestricted-stash branch from 486c5f6 to c6cc684 Compare January 13, 2023 14:29

Added support for UnrestrictedStash

92e1e16

ismaelhamed force-pushed the unrestricted-stash branch from c6cc684 to 92e1e16 Compare January 18, 2023 07:06

Aaronontheweb added this to the 1.5.0 milestone Jan 18, 2023

Aaronontheweb added akka-actor api-change labels Jan 18, 2023

Aaronontheweb approved these changes Jan 18, 2023

View reviewed changes

Aaronontheweb merged commit 7b1090d into akkadotnet:dev Jan 18, 2023

ismaelhamed added a commit to ismaelhamed/akka.net that referenced this pull request Jan 21, 2023

Added support for UnrestrictedStash (akkadotnet#6325)

35aaa62

ismaelhamed mentioned this pull request Jan 21, 2023

[BACKPORT] Several stashing improvements (#6323, #6325 and #6327) #6358

Merged

Aaronontheweb mentioned this pull request Jan 26, 2023

added v1.4.49 release notes #6369

Merged

Arkatufus mentioned this pull request Feb 1, 2023

Update RELEASE_NOTES.md for 1.5.0-alpha4 release #6379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for `UnrestrictedStash` #6325

Added support for `UnrestrictedStash` #6325

ismaelhamed commented Jan 4, 2023 •

edited

Loading

ismaelhamed Jan 4, 2023 •

edited

Loading

Aaronontheweb Jan 18, 2023

ismaelhamed Jan 4, 2023

Aaronontheweb commented Jan 4, 2023

ismaelhamed commented Jan 5, 2023 •

edited

Loading

ismaelhamed commented Jan 5, 2023

ismaelhamed commented Jan 5, 2023

ismaelhamed commented Jan 18, 2023

Aaronontheweb commented Jan 18, 2023

Aaronontheweb commented Jan 18, 2023

Aaronontheweb left a comment

Aaronontheweb Jan 18, 2023

Aaronontheweb Jan 18, 2023

Aaronontheweb Jan 18, 2023

Aaronontheweb Jan 18, 2023

ismaelhamed Jan 19, 2023

Aaronontheweb Jan 18, 2023

Added support for UnrestrictedStash #6325

Added support for UnrestrictedStash #6325

Conversation

ismaelhamed commented Jan 4, 2023 • edited Loading

Checklist

ismaelhamed Jan 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented Jan 4, 2023

ismaelhamed commented Jan 5, 2023 • edited Loading

ismaelhamed commented Jan 5, 2023

ismaelhamed commented Jan 5, 2023

ismaelhamed commented Jan 18, 2023

Aaronontheweb commented Jan 18, 2023

Aaronontheweb commented Jan 18, 2023

Aaronontheweb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Added support for `UnrestrictedStash` #6325

Added support for `UnrestrictedStash` #6325

ismaelhamed commented Jan 4, 2023 •

edited

Loading

ismaelhamed Jan 4, 2023 •

edited

Loading

ismaelhamed commented Jan 5, 2023 •

edited

Loading