Reorder ChangeState(ProcessingCommands()) and OnReplaySuccess() inside Eventsourced.Recovery.cs

[ErikOlufson]

Hi guys,

I was wondering if it would be smart to change the order inside Eventsourced.Recovery.cs.

When changing the EventsourcedState and the message is RecoverySuccess it would be nice to reorder OnReplaySuccess() and ChangeState(ProcessingCommands()).

Right now:

else if (message is RecoverySuccess)
{
    var m = (RecoverySuccess)message;
    timeoutCancelable.Cancel();
    OnReplaySuccess();
    ChangeState(ProcessingCommands());
    _sequenceNr = m.HighestSequenceNr;
    LastSequenceNr = m.HighestSequenceNr;
    _internalStash.UnstashAll();
    base.AroundReceive(recoveryBehavior, RecoveryCompleted.Instance);
    ReturnRecoveryPermit();
}

Reordered:

else if (message is RecoverySuccess)
{
    var m = (RecoverySuccess)message;
    timeoutCancelable.Cancel();
    ChangeState(ProcessingCommands());
    OnReplaySuccess();
    _sequenceNr = m.HighestSequenceNr;
    LastSequenceNr = m.HighestSequenceNr;
    _internalStash.UnstashAll();
    base.AroundReceive(recoveryBehavior, RecoveryCompleted.Instance);
    ReturnRecoveryPermit();
}

If this would be done it is possible to check variables like IsRecovering inside the overriden OnReplaySuccess() and to ensure that all recoveries are done. Right now I can check IsRecovering inside my overriden OnReplaySuccess() but the status of IsRecovering is not up to date cause this status will be set inside ChangeState(ProcessingCommands()).

What do you think about this idea?

Best regards!

[ismaelhamed]

Since OnReplaySuccess is invoked after the RecoverySuccess has been received, I think it is safe to assume that recovering is done at that point.

[Aaronontheweb]

@ismaelhamed so this is a non-issue then?

[ismaelhamed]

@Aaronontheweb the way I see it: 1) if you need to have some side-effects after recovery has completed, you can use the OnReplaySuccess --there's no need to check IsRecovering; 2) unless you want to persist, in which case the proper way would be to capture the RecoveryCompleted event in ReceiveRecover --at this point IsRecovering is false. The later is specially true since #3366.

Does it make sense, @ErikOlufson?