Align behavior of grain storage providers when setting IGrainState<T> properties (#9417)

Author: ReubenBond

src/AWS/Orleans.Persistence.DynamoDB/Provider/DynamoDBGrainStorage.cs

@@ -32,9 +32,8 @@ public class DynamoDBGrainStorage : IGrainStorage, ILifecycleParticipant<ISiloLi
         private const string CURRENT_ETAG_ALIAS = ":currentETag";
 
         private readonly DynamoDBStorageOptions options;
+        private readonly IActivatorProvider _activatorProvider;
         private readonly ILogger logger;
-        private readonly IServiceProvider serviceProvider;
-        private readonly IActivatorProvider activatorProvider;
         private readonly string name;
 
         private DynamoDBStorage storage;
@@ -45,14 +44,13 @@ public class DynamoDBGrainStorage : IGrainStorage, ILifecycleParticipant<ISiloLi
         public DynamoDBGrainStorage(
             string name,
             DynamoDBStorageOptions options,
-            IServiceProvider serviceProvider,
+            IActivatorProvider activatorProvider,
             ILogger<DynamoDBGrainStorage> logger)
         {
             this.name = name;
             this.logger = logger;
             this.options = options;
-            this.serviceProvider = serviceProvider;
-            this.activatorProvider = this.serviceProvider.GetRequiredService<IActivatorProvider>();
+            _activatorProvider = activatorProvider;
         }
 
         public void Participate(ISiloLifecycle lifecycle)
@@ -152,17 +150,13 @@ public async Task ReadStateAsync<T>(string grainType, GrainId grainId, IGrainSta
             {
                 var loadedState = ConvertFromStorageFormat<T>(record);
                 grainState.RecordExists = loadedState != null;
-                grainState.State = loadedState ?? Activator.CreateInstance<T>();
+                grainState.State = loadedState ?? CreateInstance<T>();
                 grainState.ETag = record.ETag.ToString();
             }
             else
             {
-                grainState.RecordExists = false;
-                grainState.ETag = null;
-                grainState.State = this.activatorProvider.GetActivator<T>().Create();
+                ResetGrainState(grainState);
             }
-
-            // Else leave grainState in previous default condition
         }
 
         /// <summary> Write state data function for this storage provider. </summary>
@@ -190,7 +184,7 @@ public async Task WriteStateAsync<T>(string grainType, GrainId grainId, IGrainSt
                 this.logger.LogError(
                     (int)ErrorCode.StorageProviderBase,
                     exc,
-                    "Error Writing: GrainType={GrainType} Grainid={GrainId} ETag={ETag} to Table={TableName}",
+                    "Error Writing: GrainType={GrainType} GrainId={GrainId} ETag={ETag} to Table={TableName}",
                     grainType,
                     grainId,
                     grainState.ETag,
@@ -298,12 +292,13 @@ public async Task ClearStateAsync<T>(string grainType, GrainId grainId, IGrainSt
                     keys.Add(GRAIN_TYPE_PROPERTY_NAME, new AttributeValue(record.GrainType));
 
                     await this.storage.DeleteEntryAsync(this.options.TableName, keys).ConfigureAwait(false);
-                    grainState.ETag = null;
+                    ResetGrainState(grainState);
                 }
                 else
                 {
                     await WriteStateInternal(grainState, record, true);
-                    grainState.State = this.activatorProvider.GetActivator<T>().Create();
+                    grainState.State = CreateInstance<T>();
+                    grainState.RecordExists = false;
                 }
             }
             catch (Exception exc)
@@ -381,6 +376,15 @@ internal void ConvertToStorageFormat(object grainState, GrainStateRecord entity)
                 throw new ArgumentOutOfRangeException("GrainState.Size", msg);
             }
         }
+
+        private void ResetGrainState<T>(IGrainState<T> grainState)
+        {
+            grainState.RecordExists = false;
+            grainState.ETag = null;
+            grainState.State = CreateInstance<T>();
+        }
+
+        private T CreateInstance<T>() => _activatorProvider.GetActivator<T>().Create();
     }
 
     public static class DynamoDBGrainStorageFactory

src/AdoNet/Orleans.Persistence.AdoNet/Storage/Provider/AdoNetGrainStorage.cs

@@ -16,6 +16,7 @@
 using Orleans.Configuration.Overrides;
 using Orleans.Configuration;
 using Orleans.Runtime.Configuration;
+using Orleans.Serialization.Serializers;
 
 namespace Orleans.Storage
 {
@@ -69,7 +70,7 @@ public static AdoNetGrainStorage Create(IServiceProvider services, string name)
     /// </para>
     /// </remarks>
     [DebuggerDisplay("Name = {Name}, ConnectionString = {Storage.ConnectionString}")]
-    public class AdoNetGrainStorage: IGrainStorage, ILifecycleParticipant<ISiloLifecycle>
+    public class AdoNetGrainStorage : IGrainStorage, ILifecycleParticipant<ISiloLifecycle>
     {
         public IGrainStorageSerializer Serializer { get; set; }
 
@@ -90,7 +91,7 @@ public class AdoNetGrainStorage: IGrainStorage, ILifecycleParticipant<ISiloLifec
         /// The Service ID for which this relational provider is used.
         /// </summary>
         private readonly string serviceId;
-
+        private readonly IActivatorProvider _activatorProvider;
         private readonly ILogger logger;
 
         /// <summary>
@@ -121,29 +122,29 @@ public class AdoNetGrainStorage: IGrainStorage, ILifecycleParticipant<ISiloLifec
         public IStorageHasherPicker HashPicker { get; set; }
 
         private readonly AdoNetGrainStorageOptions options;
-        private readonly IProviderRuntime providerRuntime;
         private readonly string name;
 
         public AdoNetGrainStorage(
+            IActivatorProvider activatorProvider,
             ILogger<AdoNetGrainStorage> logger,
-            IProviderRuntime providerRuntime,
             IOptions<AdoNetGrainStorageOptions> options,
             IOptions<ClusterOptions> clusterOptions,
             string name)
         {
             this.options = options.Value;
-            this.providerRuntime = providerRuntime;
             this.name = name;
+            _activatorProvider = activatorProvider;
             this.logger = logger;
             this.serviceId = clusterOptions.Value.ServiceId;
             this.Serializer = options.Value.GrainStorageSerializer;
-            this.HashPicker = options.Value.HashPicker ?? new StorageHasherPicker(new[] { new OrleansDefaultHasher() });;
+            this.HashPicker = options.Value.HashPicker ?? new StorageHasherPicker(new[] { new OrleansDefaultHasher() });
         }
 
         public void Participate(ISiloLifecycle lifecycle)
         {
             lifecycle.Subscribe(OptionFormattingUtilities.Name<AdoNetGrainStorage>(this.name), this.options.InitStage, Init, Close);
         }
+
         /// <summary>Clear state data function for this storage provider.</summary>
         /// <see cref="IGrainStorage.ClearStateAsync{T}"/>.
         public async Task ClearStateAsync<T>(string grainType, GrainId grainReference, IGrainState<T> grainState)
@@ -152,7 +153,7 @@ public async Task ClearStateAsync<T>(string grainType, GrainId grainReference, I
             //even if not as clear as when using explicitly checked parameters.
             var grainId = GrainIdAndExtensionAsString(grainReference);
             var baseGrainType = ExtractBaseClass(grainType);
-            if(logger.IsEnabled(LogLevel.Trace))
+            if (logger.IsEnabled(LogLevel.Trace))
             {
                 logger.LogTrace(
                     (int)RelationalStorageProviderCodes.RelationalProviderClearing,
@@ -164,6 +165,15 @@ public async Task ClearStateAsync<T>(string grainType, GrainId grainReference, I
                     grainState.ETag);
             }
 
+            if (!grainState.RecordExists)
+            {
+                await ReadStateAsync(grainType, grainReference, grainState).ConfigureAwait(false);
+                if (!grainState.RecordExists)
+                {
+                    return;
+                }
+            }
+
             string storageVersion = null;
             try
             {
@@ -182,7 +192,7 @@ public async Task ClearStateAsync<T>(string grainType, GrainId grainReference, I
                 }, (selector, resultSetCount, token) => Task.FromResult(selector.GetValue(0).ToString()), cancellationToken: CancellationToken.None).ConfigureAwait(false));
                 storageVersion = clearRecord.SingleOrDefault();
             }
-            catch(Exception ex)
+            catch (Exception ex)
             {
                 logger.LogError(
                     (int)RelationalStorageProviderCodes.RelationalProviderDeleteError,
@@ -198,15 +208,16 @@ public async Task ClearStateAsync<T>(string grainType, GrainId grainReference, I
 
             const string OperationString = "ClearState";
             var inconsistentStateException = CheckVersionInconsistency(OperationString, serviceId, this.name, storageVersion, grainState.ETag, baseGrainType, grainId.ToString());
-            if(inconsistentStateException != null)
+            if (inconsistentStateException != null)
             {
                 throw inconsistentStateException;
             }
 
             //No errors found, the version of the state held by the grain can be updated and also the state.
             grainState.ETag = storageVersion;
             grainState.RecordExists = false;
-            if(logger.IsEnabled(LogLevel.Trace))
+            grainState.State = CreateInstance<T>();
+            if (logger.IsEnabled(LogLevel.Trace))
             {
                 logger.LogTrace(
                     (int)RelationalStorageProviderCodes.RelationalProviderCleared,
@@ -273,20 +284,24 @@ public async Task ReadStateAsync<T>(string grainType, GrainId grainReference, IG
                     },
                     commandBehavior, CancellationToken.None).ConfigureAwait(false)).SingleOrDefault();
 
-                T state = readRecords != null ? (T) readRecords.Item1 : default;
+                T state = readRecords != null ? (T)readRecords.Item1 : default;
                 string etag = readRecords != null ? readRecords.Item2 : null;
                 bool recordExists = readRecords != null;
-                if(state == null)
+                if (state == null)
                 {
-                    logger.LogInformation(
-                        (int)RelationalStorageProviderCodes.RelationalProviderNoStateFound,
-                        "Null grain state read (default will be instantiated): ServiceId={ServiceId} ProviderName={Name} GrainType={BaseGrainType} GrainId={GrainId} ETag={ETag}.",
-                        serviceId,
-                        name,
-                        baseGrainType,
-                        grainId,
-                        grainState.ETag);
-                    state = Activator.CreateInstance<T>();
+                    if (logger.IsEnabled(LogLevel.Trace))
+                    {
+                        logger.LogTrace(
+                            (int)RelationalStorageProviderCodes.RelationalProviderNoStateFound,
+                            "Null grain state read (default will be instantiated): ServiceId={ServiceId} ProviderName={Name} GrainType={BaseGrainType} GrainId={GrainId} ETag={ETag}.",
+                            serviceId,
+                            name,
+                            baseGrainType,
+                            grainId,
+                            grainState.ETag);
+                    }
+
+                    state = CreateInstance<T>();
                 }
 
                 grainState.State = state;
@@ -304,7 +319,7 @@ public async Task ReadStateAsync<T>(string grainType, GrainId grainReference, IG
                         grainState.ETag);
                 }
             }
-            catch(Exception ex)
+            catch (Exception ex)
             {
                 logger.LogError(
                     (int)RelationalStorageProviderCodes.RelationalProviderReadError,
@@ -363,7 +378,7 @@ public async Task WriteStateAsync<T>(string grainType, GrainId grainReference, I
                 { return Task.FromResult(selector.GetNullableInt32("NewGrainStateVersion").ToString()); }, cancellationToken: CancellationToken.None).ConfigureAwait(false);
                 storageVersion = writeRecord.SingleOrDefault();
             }
-            catch(Exception ex)
+            catch (Exception ex)
             {
                 logger.LogError(
                     (int)RelationalStorageProviderCodes.RelationalProviderWriteError,
@@ -379,7 +394,7 @@ public async Task WriteStateAsync<T>(string grainType, GrainId grainReference, I
 
             const string OperationString = "WriteState";
             var inconsistentStateException = CheckVersionInconsistency(OperationString, serviceId, this.name, storageVersion, grainState.ETag, baseGrainType, grainId.ToString());
-            if(inconsistentStateException != null)
+            if (inconsistentStateException != null)
             {
                 throw inconsistentStateException;
             }
@@ -424,7 +439,6 @@ private async Task Init(CancellationToken cancellationToken)
                 ConfigUtilities.RedactConnectionStringInfo(Storage.ConnectionString));
         }
 
-
         /// <summary>
         /// Close this provider
         /// </summary>
@@ -433,7 +447,6 @@ private Task Close(CancellationToken token)
             return Task.CompletedTask;
         }
 
-
         /// <summary>
         /// Checks for version inconsistency as defined in the database scripts.
         /// </summary>
@@ -455,7 +468,7 @@ private static InconsistentStateException CheckVersionInconsistency(string opera
             //it means two grains were activated an the other one succeeded in writing its state.
             //
             //NOTE: the storage could return also the new and old ETag (Version), but currently it doesn't.
-            if(storageVersion == grainVersion || storageVersion == string.Empty)
+            if (storageVersion == grainVersion || storageVersion == string.Empty)
             {
                 //TODO: Note that this error message should be canonical across back-ends.
                 return new InconsistentStateException($"Version conflict ({operation}): ServiceId={serviceId} ProviderName={providerName} GrainType={normalizedGrainType} GrainId={grainId} ETag={grainVersion}.");
@@ -545,5 +558,7 @@ int GetGenericArity(string input)
                 return 0;
             }
         }
+
+        private T CreateInstance<T>() => _activatorProvider.GetActivator<T>().Create();
     }
 }