Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActorPath changes between ActorRef and ActorGrain #181

Closed
Jubast opened this issue Sep 18, 2023 · 4 comments · Fixed by #183
Closed

ActorPath changes between ActorRef and ActorGrain #181

Jubast opened this issue Sep 18, 2023 · 4 comments · Fixed by #183

Comments

@Jubast
Copy link
Contributor

Jubast commented Sep 18, 2023

When upgrading from Orleankka 3.x to 7.0.0 i've noticed that ActorPath.Id property changes between the client side and grain side

Example code:

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Orleankka;
using Orleankka.Cluster;
using Orleankka.Meta;

var host = await new HostBuilder()    
    .UseOrleans(orleans => orleans.UseLocalhostClustering())
    .UseOrleankka()
    .StartAsync();

var system = host.Services.GetService<IActorSystem>();
var @ref = system.ActorOf<ITestActor>("myId");

Console.WriteLine($"ActorPath on client: {@ref.Path}");

await @ref.Tell(new TestCommand());
await host.StopAsync();

public interface ITestActor : IActorGrain, IGrainWithStringKey { }

public class TestActor : DispatchActorGrain, ITestActor
{
    public void Handle(TestCommand _) => Console.WriteLine($"ActorPath on grain: {Path}");
}

[GenerateSerializer]
public class TestCommand : Command { }

Output:

ActorPath on client: ITestActor:myId
ActorPath on grain: ITestActor:testactor/myId

This happens because of the current ActorGrain initialization logic where the whole GrainId(GrainType + Key) gets forwarded the actor id:

actor.Initialize(context.ActivationServices, context.GrainId.ToString());

I believe this is unintended, would you accept a change request so that only GrainId.Key gets forwarded as the id?

@yevhen
Copy link
Member

yevhen commented Oct 14, 2023

This was a change in id scheme in upstream project. But you can try with PR. If tests wouldn’t break then we are good to go

@Jubast
Copy link
Contributor Author

Jubast commented Oct 17, 2023

I do understand that, but this project also needs to be adjusted for that. We later discovered that because of this every Actor.Self call is now broken because messages are sent to a different actor.

Example code:

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Orleankka;
using Orleankka.Cluster;
using Orleankka.Meta;

var host = await new HostBuilder()
    .UseOrleans(orleans => orleans.UseLocalhostClustering())
    .UseOrleankka()
    .StartAsync();

var system = host.Services.GetService<IActorSystem>();
var @ref = system.ActorOf<ITestActor>("myId");

Console.WriteLine($"ActorPath on client: {@ref.Path}");

await @ref.Tell(new TestRepeatCommand());
await host.StopAsync();

public interface ITestActor : IActorGrain, IGrainWithStringKey { }

public class TestActor : DispatchActorGrain, ITestActor
{
    public override Task OnActivateAsync(CancellationToken cancellationToken)
    {
        Console.WriteLine($"Actor: {Path} activated");
        return base.OnActivateAsync(cancellationToken);
    }

    public void Handle(TestCommand _) => Console.WriteLine($"Actor: {Path} processed command");

    public void Handle(TestRepeatCommand _)
    {
        Console.WriteLine($"Actor: {Path} processed command");
        Self.Notify(new TestCommand());
    }
}

[GenerateSerializer]
public class TestCommand : Command { }

[GenerateSerializer]
public class TestRepeatCommand : Command { }

Output:

ActorPath on client: ITestActor:myId
Actor: ITestActor:testactor/myId activated
Actor: ITestActor:testactor/myId processed command
Actor: ITestActor:testactor/testactor/myId activated
Actor: ITestActor:testactor/testactor/myId processed command

I will prepare a PR

@yevhen
Copy link
Member

yevhen commented Nov 11, 2023

7.0.1 on nuget

@Jubast
Copy link
Contributor Author

Jubast commented Nov 13, 2023

Thank you! :)

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 a pull request may close this issue.

2 participants