0

What seems to be just common practice could be the wrong thing to do in Service Fabric. I suspect the below code where stateManager is saved as local cache could cause a potential issue when the 'Startup' class is instantiated within the return statement of 'CreateServiceReplicaListeners()' method in 'SomeService' stateful service.

The situation that can happen is when the state manager is somehow re-instantiated. I need more explanation as to whether the below practice is the right thing to do or not. If not, what could be the best practice instead?

internal class SomeService : StatefulService
{
    protected  override IEnumerable<ServiceReplicaListener> CreateServiceReplicaListeners()
    {
        return new[]{
            new ServiceReplicaListener(
                        initParams =>
                            new OwinCommunicationListener("SomeService", new Startup(this.StateManager), initParams))
                };
        }
    }
}

public class Startup : IOwinAppBuilder
{
    private readonly IReliableStateManager stateManager;

    public Startup(IReliableStateManager stateManager)
    {
        this.stateManager = stateManager;
    }

    public void Configuration(IAppBuilder appBuilder)
    {
        // other initialization codes..
        ...
        ...

        UnityConfig.RegisterComponents(config, this.stateManager);

        appBuilder.UseWebApi(config);
    }
}
Super Jade
  • 5,609
  • 7
  • 39
  • 61
swcraft
  • 2,014
  • 3
  • 22
  • 43
  • Can your question a bit. Are you asking whether passing `IReliableStateManager` into startup is *OK* or you have received an exception or undefined behavior when doing that? – Oleg Karasik Feb 15 '19 at 05:50

1 Answers1

2

Whenever a Stateful Service change roles it triggers a IStatefulServiceReplica.ChangeRoleAsync(ReplicaRole newRole, CancellationToken cancellationToken).

ChangeRoleAsync(..) ensure that the new role uses the correct communications doing the following:

  • Call CloseCommunicationListenersAsync(CancellationToken cancellationToken) to close any listeners open
  • Call OpenCommunicationListenersAsync(newRole, cancellationToken) for Primary or ActiveSecondary roles
  • The method OpenCommunicationListenersAsync() will call CreateServiceReplicaListeners() to get the listeners and call CreateCommunicationListener(serviceContext) for each returned listener to open the related endpoints.

Change of Roles is very common to happen during upgrades and Load Balancing, so this is a very common event.

In Summary,

Every time a Change of Role happens, CreateServiceReplicaListeners() will be called, ChangeRole does not shutdown the service, so it might have side effects, for example if you register dependencies in a DI container, you might face duplicate registrations.

Diego Mendes
  • 10,631
  • 2
  • 32
  • 36
  • Thanks, so do you mean this line of code "UnityConfig.RegisterComponents(config, this.stateManager);" could be containing the obsolete reference of stateManager if I understood correctly. – swcraft Feb 15 '19 at 19:01
  • And during the 'OpenCommunicationListenerAsync() which will call CreateServiceReplicaListerns(), would the 'this.StateManager' be still valid? As Stateful Service change roles, I am afraid if the local cache 'stateManager' inside Startup class would be stale and we need new copy.. – swcraft Feb 15 '19 at 19:19
  • 1
    @JP_medevice The instance of `this.StateManager` will be valid (it is not reinitialized when the roles are changed). – Oleg Karasik Feb 18 '19 at 06:08
  • 1
    AFAIK, the StateManager should be the same during the lifetime of the Service, if you are facing any issues, is likely because the StateManager is being registered in the DI and conflicting with something. Take a look t this queston: https://stackoverflow.com/a/54197276/484222 – Diego Mendes Feb 18 '19 at 09:57