Skip to content

Commit 005d8c6

Browse files
ReubenBondReubenBond
committed
Review suggestions
1 parent 3c8a962 commit 005d8c6

20 files changed

+246
-184
lines changed

‎src/Orleans.Core.Abstractions/Placement/PlacementFilterAttribute.cs

+3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ namespace Orleans.Placement;
1212
[AttributeUsage(AttributeTargets.Class, AllowMultiple = true)]
1313
public abstract class PlacementFilterAttribute : Attribute, IGrainPropertiesProviderAttribute
1414
{
15+
/// <summary>
16+
/// Gets the placement filter strategy.
17+
/// </summary>
1518
public PlacementFilterStrategy PlacementFilterStrategy { get; private set; }
1619

1720
protected PlacementFilterAttribute(PlacementFilterStrategy placement)

‎src/Orleans.Core.Abstractions/Placement/PlacementFilterStrategy.cs

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Globalization;
34
using Orleans.Metadata;
45
using Orleans.Runtime;
56

67
#nullable enable
78
namespace Orleans.Placement;
89

10+
/// <summary>
11+
/// Represents a strategy for filtering silos which a grain can be placed on.
12+
/// </summary>
913
public abstract class PlacementFilterStrategy
1014
{
1115
public int Order { get; private set; }
@@ -36,7 +40,6 @@ public void Initialize(GrainProperties properties)
3640

3741
public virtual void AdditionalInitialize(GrainProperties properties)
3842
{
39-
4043
}
4144

4245
/// <summary>
@@ -58,7 +61,7 @@ public void PopulateGrainProperties(IServiceProvider services, Type grainClass,
5861
properties[WellKnownGrainTypeProperties.PlacementFilter] = typeName;
5962
}
6063

61-
properties[$"{WellKnownGrainTypeProperties.PlacementFilter}.{typeName}.order"] = Order.ToString();
64+
properties[$"{WellKnownGrainTypeProperties.PlacementFilter}.{typeName}.order"] = Order.ToString(CultureInfo.InvariantCulture);
6265

6366
foreach (var additionalGrainProperty in GetAdditionalGrainProperties(services, grainClass, grainType, properties))
6467
{

‎src/Orleans.Core/Runtime/Constants.cs

+5-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ internal static class Constants
1212
public static readonly GrainType DirectoryCacheValidatorType = SystemTargetGrainId.CreateGrainType("dir.cache-validator");
1313
public static readonly GrainType ClientDirectoryType = SystemTargetGrainId.CreateGrainType("dir.client");
1414
public static readonly GrainType SiloControlType = SystemTargetGrainId.CreateGrainType("silo-control");
15+
public static readonly GrainType SiloMetadataType = SystemTargetGrainId.CreateGrainType("silo-metadata");
1516
public static readonly GrainType CatalogType = SystemTargetGrainId.CreateGrainType("catalog");
1617
public static readonly GrainType MembershipServiceType = SystemTargetGrainId.CreateGrainType("clustering");
1718
public static readonly GrainType SystemMembershipTableType = SystemTargetGrainId.CreateGrainType("clustering.dev");
@@ -27,8 +28,8 @@ internal static class Constants
2728
public static readonly GrainType ActivationMigratorType = SystemTargetGrainId.CreateGrainType("migrator");
2829
public static readonly GrainType ActivationRepartitionerType = SystemTargetGrainId.CreateGrainType("repartitioner");
2930
public static readonly GrainType ActivationRebalancerMonitorType = SystemTargetGrainId.CreateGrainType("rebalancer-monitor");
30-
public static readonly GrainType GrainDirectoryPartition = SystemTargetGrainId.CreateGrainType("dir.grain.part");
31-
public static readonly GrainType GrainDirectory = SystemTargetGrainId.CreateGrainType("dir.grain");
31+
public static readonly GrainType GrainDirectoryPartitionType = SystemTargetGrainId.CreateGrainType("dir.grain.part");
32+
public static readonly GrainType GrainDirectoryType = SystemTargetGrainId.CreateGrainType("dir.grain");
3233

3334
public static readonly GrainId SiloDirectConnectionId = GrainId.Create(
3435
GrainType.Create(GrainTypePrefix.SystemPrefix + "silo"),
@@ -41,6 +42,7 @@ internal static class Constants
4142
{DirectoryServiceType, "DirectoryService"},
4243
{DirectoryCacheValidatorType, "DirectoryCacheValidator"},
4344
{SiloControlType, "SiloControl"},
45+
{SiloMetadataType, "SiloMetadata"},
4446
{ClientDirectoryType, "ClientDirectory"},
4547
{CatalogType,"Catalog"},
4648
{MembershipServiceType,"MembershipService"},
@@ -57,7 +59,7 @@ internal static class Constants
5759
{ActivationMigratorType, "ActivationMigrator"},
5860
{ActivationRepartitionerType, "ActivationRepartitioner"},
5961
{ActivationRebalancerMonitorType, "ActivationRebalancerMonitor"},
60-
{GrainDirectory, "GrainDirectory"},
62+
{GrainDirectoryType, "GrainDirectory"},
6163
}.ToFrozenDictionary();
6264

6365
public static string SystemTargetName(GrainType id) => SingletonSystemTargetNames.TryGetValue(id, out var name) ? name : id.ToString();

‎src/Orleans.Runtime/GrainDirectory/DistributedGrainDirectory.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public DistributedGrainDirectory(
8484
ILocalSiloDetails localSiloDetails,
8585
ILoggerFactory loggerFactory,
8686
IServiceProvider serviceProvider,
87-
IInternalGrainFactory grainFactory) : base(Constants.GrainDirectory, localSiloDetails.SiloAddress, loggerFactory)
87+
IInternalGrainFactory grainFactory) : base(Constants.GrainDirectoryType, localSiloDetails.SiloAddress, loggerFactory)
8888
{
8989
_serviceProvider = serviceProvider;
9090
_membershipService = membershipService;

‎src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ internal sealed partial class GrainDirectoryPartition(
2929
IInternalGrainFactory grainFactory)
3030
: SystemTarget(CreateGrainId(localSiloDetails.SiloAddress, partitionIndex), localSiloDetails.SiloAddress, loggerFactory), IGrainDirectoryPartition, IGrainDirectoryTestHooks
3131
{
32-
internal static SystemTargetGrainId CreateGrainId(SiloAddress siloAddress, int partitionIndex) => SystemTargetGrainId.Create(Constants.GrainDirectoryPartition, siloAddress, partitionIndex.ToString(CultureInfo.InvariantCulture));
32+
internal static SystemTargetGrainId CreateGrainId(SiloAddress siloAddress, int partitionIndex) => SystemTargetGrainId.Create(Constants.GrainDirectoryPartitionType, siloAddress, partitionIndex.ToString(CultureInfo.InvariantCulture));
3333
private readonly Dictionary<GrainId, GrainAddress> _directory = [];
3434
private readonly int _partitionIndex = partitionIndex;
3535
private readonly DistributedGrainDirectory _owner = owner;
@@ -665,7 +665,7 @@ private async IAsyncEnumerable<List<GrainAddress>> GetRegisteredActivations(Dire
665665
async Task<List<GrainAddress>> GetRegisteredActivationsFromClusterMember(MembershipVersion version, RingRange range, SiloAddress siloAddress, bool isValidation)
666666
{
667667
var stopwatch = ValueStopwatch.StartNew();
668-
var client = _grainFactory.GetSystemTarget<IGrainDirectoryClient>(Constants.GrainDirectory, siloAddress);
668+
var client = _grainFactory.GetSystemTarget<IGrainDirectoryClient>(Constants.GrainDirectoryType, siloAddress);
669669
var result = await InvokeOnClusterMember(
670670
siloAddress,
671671
async () =>

‎src/Orleans.Runtime/MembershipService/SiloMetadata/ISiloMetadataCache.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#nullable enable
2+
23
namespace Orleans.Runtime.MembershipService.SiloMetadata;
34

45
public interface ISiloMetadataCache
56
{
6-
SiloMetadata GetMetadata(SiloAddress siloAddress);
7+
SiloMetadata GetSiloMetadata(SiloAddress siloAddress);
78
}

‎src/Orleans.Runtime/MembershipService/SiloMetadata/ISiloMetadataClient.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
using System.Threading.Tasks;
2-
using Orleans.Services;
32

43
#nullable enable
54
namespace Orleans.Runtime.MembershipService.SiloMetadata;
65

7-
public interface ISiloMetadataClient : IGrainServiceClient<ISiloMetadataGrainService>
6+
internal interface ISiloMetadataClient
87
{
98
Task<SiloMetadata> GetSiloMetadata(SiloAddress siloAddress);
109
}

‎src/Orleans.Runtime/MembershipService/SiloMetadata/ISiloMetadataGrainService.cs ‎src/Orleans.Runtime/MembershipService/SiloMetadata/ISiloMetadataSystemTarget.cs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
using System.Threading.Tasks;
2-
using Orleans.Services;
32

43
#nullable enable
54
namespace Orleans.Runtime.MembershipService.SiloMetadata;
65

7-
[Alias("Orleans.Runtime.MembershipService.SiloMetadata.ISiloMetadataGrainService")]
8-
public interface ISiloMetadataGrainService : IGrainService
6+
[Alias("Orleans.Runtime.MembershipService.SiloMetadata.ISiloMetadataSystemTarget")]
7+
internal interface ISiloMetadataSystemTarget : ISystemTarget
98
{
109
[Alias("GetSiloMetadata")]
1110
Task<SiloMetadata> GetSiloMetadata();

‎src/Orleans.Runtime/MembershipService/SiloMetadata/SiloMetadaCache.cs

+20-18
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
using System.Threading;
66
using System.Threading.Tasks;
77
using Microsoft.Extensions.Logging;
8-
using Orleans.Configuration;
9-
using Orleans.Internal;
108

119
#nullable enable
1210
namespace Orleans.Runtime.MembershipService.SiloMetadata;
@@ -22,44 +20,44 @@ internal class SiloMetadataCache(
2220

2321
void ILifecycleParticipant<ISiloLifecycle>.Participate(ISiloLifecycle lifecycle)
2422
{
25-
var tasks = new List<Task>(1);
26-
var cancellation = new CancellationTokenSource();
27-
Task OnRuntimeInitializeStart(CancellationToken _)
23+
Task? task = null;
24+
Task OnStart(CancellationToken _)
2825
{
29-
tasks.Add(Task.Run(() => this.ProcessMembershipUpdates(cancellation.Token)));
26+
task = Task.Run(() => this.ProcessMembershipUpdates(_cts.Token));
3027
return Task.CompletedTask;
3128
}
3229

33-
async Task OnRuntimeInitializeStop(CancellationToken ct)
30+
async Task OnStop(CancellationToken ct)
3431
{
35-
cancellation.Cancel(throwOnFirstException: false);
36-
var shutdownGracePeriod = Task.WhenAll(Task.Delay(ClusterMembershipOptions.ClusteringShutdownGracePeriod), ct.WhenCancelled());
37-
await Task.WhenAny(shutdownGracePeriod, Task.WhenAll(tasks));
32+
await _cts.CancelAsync().ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
33+
if (task is not null)
34+
{
35+
await task.WaitAsync(ct).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
36+
}
3837
}
3938

4039
lifecycle.Subscribe(
4140
nameof(ClusterMembershipService),
42-
ServiceLifecycleStage.RuntimeInitialize,
43-
OnRuntimeInitializeStart,
44-
OnRuntimeInitializeStop);
41+
ServiceLifecycleStage.RuntimeServices,
42+
OnStart,
43+
OnStop);
4544
}
4645

47-
4846
private async Task ProcessMembershipUpdates(CancellationToken ct)
4947
{
5048
try
5149
{
52-
if (logger.IsEnabled(LogLevel.Debug)) logger.LogDebug("Starting to process membership updates");
50+
if (logger.IsEnabled(LogLevel.Debug)) logger.LogDebug("Starting to process membership updates.");
5351
await foreach (var update in membershipTableManager.MembershipTableUpdates.WithCancellation(ct))
5452
{
5553
// Add entries for members that aren't already in the cache
56-
foreach (var membershipEntry in update.Entries.Where(e => e.Value.Status != SiloStatus.Dead))
54+
foreach (var membershipEntry in update.Entries.Where(e => e.Value.Status is SiloStatus.Active or SiloStatus.Joining))
5755
{
5856
if (!_metadata.ContainsKey(membershipEntry.Key))
5957
{
6058
try
6159
{
62-
var metadata = await siloMetadataClient.GetSiloMetadata(membershipEntry.Key);
60+
var metadata = await siloMetadataClient.GetSiloMetadata(membershipEntry.Key).WaitAsync(ct);
6361
_metadata.TryAdd(membershipEntry.Key, metadata);
6462
}
6563
catch(Exception exception)
@@ -85,6 +83,10 @@ private async Task ProcessMembershipUpdates(CancellationToken ct)
8583
}
8684
}
8785
}
86+
catch (OperationCanceledException) when (ct.IsCancellationRequested)
87+
{
88+
// Ignore and continue shutting down.
89+
}
8890
catch (Exception exception)
8991
{
9092
logger.LogError(exception, "Error processing membership updates");
@@ -95,7 +97,7 @@ private async Task ProcessMembershipUpdates(CancellationToken ct)
9597
}
9698
}
9799

98-
public SiloMetadata GetMetadata(SiloAddress siloAddress) => _metadata.GetValueOrDefault(siloAddress) ?? SiloMetadata.Empty;
100+
public SiloMetadata GetSiloMetadata(SiloAddress siloAddress) => _metadata.GetValueOrDefault(siloAddress) ?? SiloMetadata.Empty;
99101

100102
public void SetMetadata(SiloAddress siloAddress, SiloMetadata metadata) => _metadata.TryAdd(siloAddress, metadata);
101103

‎src/Orleans.Runtime/MembershipService/SiloMetadata/SiloMetadataClient.cs

+3-6
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
1-
using System;
21
using System.Threading.Tasks;
3-
using Orleans.Runtime.Services;
42

53
#nullable enable
64
namespace Orleans.Runtime.MembershipService.SiloMetadata;
75

8-
public class SiloMetadataClient(IServiceProvider serviceProvider)
9-
: GrainServiceClient<ISiloMetadataGrainService>(serviceProvider), ISiloMetadataClient
6+
internal sealed class SiloMetadataClient(IInternalGrainFactory grainFactory) : ISiloMetadataClient
107
{
118
public async Task<SiloMetadata> GetSiloMetadata(SiloAddress siloAddress)
129
{
13-
var grainService = GetGrainService(siloAddress);
14-
var metadata = await grainService.GetSiloMetadata();
10+
var metadataSystemTarget = grainFactory.GetSystemTarget<ISiloMetadataSystemTarget>(Constants.SiloMetadataType, siloAddress);
11+
var metadata = await metadataSystemTarget.GetSiloMetadata();
1512
return metadata;
1613
}
1714
}

‎src/Orleans.Runtime/MembershipService/SiloMetadata/SiloMetadataGrainService.cs

+21-10
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,34 @@
1+
using System;
2+
using System.Threading;
13
using System.Threading.Tasks;
4+
using Microsoft.Extensions.DependencyInjection;
25
using Microsoft.Extensions.Logging;
36
using Microsoft.Extensions.Options;
47

58
#nullable enable
69
namespace Orleans.Runtime.MembershipService.SiloMetadata;
710

8-
public class SiloMetadataGrainService : GrainService, ISiloMetadataGrainService
11+
internal sealed class SiloMetadataSystemTarget(
12+
IOptions<SiloMetadata> siloMetadata,
13+
ILocalSiloDetails localSiloDetails,
14+
ILoggerFactory loggerFactory,
15+
IServiceProvider serviceProvider)
16+
: SystemTarget(Constants.SiloMetadataType, localSiloDetails.SiloAddress, loggerFactory), ISiloMetadataSystemTarget, ILifecycleParticipant<ISiloLifecycle>
917
{
10-
private readonly SiloMetadata _siloMetadata;
18+
private readonly SiloMetadata _siloMetadata = siloMetadata.Value;
1119

12-
public SiloMetadataGrainService(IOptions<SiloMetadata> siloMetadata) : base()
13-
{
14-
_siloMetadata = siloMetadata.Value;
15-
}
20+
public Task<SiloMetadata> GetSiloMetadata() => Task.FromResult(_siloMetadata);
1621

17-
public SiloMetadataGrainService(IOptions<SiloMetadata> siloMetadata, GrainId grainId, Silo silo, ILoggerFactory loggerFactory) : base(grainId, silo, loggerFactory)
22+
void ILifecycleParticipant<ISiloLifecycle>.Participate(ISiloLifecycle lifecycle)
1823
{
19-
_siloMetadata = siloMetadata.Value;
20-
}
24+
lifecycle.Subscribe(nameof(SiloMetadataSystemTarget), ServiceLifecycleStage.RuntimeInitialize, OnRuntimeInitializeStart, OnRuntimeInitializeStop);
2125

22-
public Task<SiloMetadata> GetSiloMetadata() => Task.FromResult(_siloMetadata);
26+
Task OnRuntimeInitializeStart(CancellationToken token)
27+
{
28+
serviceProvider.GetRequiredService<Catalog>().RegisterSystemTarget(this);
29+
return Task.CompletedTask;
30+
}
31+
32+
Task OnRuntimeInitializeStop(CancellationToken token) => Task.CompletedTask;
33+
}
2334
}

‎src/Orleans.Runtime/MembershipService/SiloMetadata/SiloMetadataHostingExtensions.cs

+5-7
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ namespace Orleans.Runtime.MembershipService.SiloMetadata;
1111

1212
public static class SiloMetadataHostingExtensions
1313
{
14-
1514
/// <summary>
1615
/// Configure silo metadata from the builder configuration.
1716
/// </summary>
@@ -58,7 +57,7 @@ public static ISiloBuilder UseSiloMetadata(this ISiloBuilder builder, IConfigura
5857
{
5958
var dictionary = configurationSection.Get<Dictionary<string, string>>();
6059

61-
return builder.UseSiloMetadata(dictionary ?? new Dictionary<string, string>());
60+
return builder.UseSiloMetadata(dictionary ?? []);
6261
}
6362

6463
/// <summary>
@@ -73,16 +72,15 @@ public static ISiloBuilder UseSiloMetadata(this ISiloBuilder builder, Dictionary
7372
{
7473
services
7574
.AddOptionsWithValidateOnStart<SiloMetadata>()
76-
.Configure(m =>
77-
{
78-
m.AddMetadata(metadata);
79-
});
75+
.Configure(m => m.AddMetadata(metadata));
8076

81-
services.AddGrainService<SiloMetadataGrainService>();
77+
services.AddSingleton<SiloMetadataSystemTarget>();
78+
services.AddFromExisting<ILifecycleParticipant<ISiloLifecycle>, SiloMetadataSystemTarget>();
8279
services.AddSingleton<SiloMetadataCache>();
8380
services.AddFromExisting<ISiloMetadataCache, SiloMetadataCache>();
8481
services.AddFromExisting<ILifecycleParticipant<ISiloLifecycle>, SiloMetadataCache>();
8582
services.AddSingleton<ISiloMetadataClient, SiloMetadataClient>();
83+
8684
// Placement filters
8785
services.AddPlacementFilter<PreferredMatchSiloMetadataPlacementFilterStrategy, PreferredMatchSiloMetadataPlacementFilterDirector>(ServiceLifetime.Transient);
8886
services.AddPlacementFilter<RequiredMatchSiloMetadataPlacementFilterStrategy, RequiredMatchSiloMetadataPlacementFilterDirector>(ServiceLifetime.Transient);

‎src/Orleans.Runtime/Placement/Filtering/PreferredMatchSiloMetadataPlacementFilterDirector.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ internal class PreferredMatchSiloMetadataPlacementFilterDirector(
1414
{
1515
public IEnumerable<SiloAddress> Filter(PlacementFilterStrategy filterStrategy, PlacementTarget target, IEnumerable<SiloAddress> silos)
1616
{
17-
var preferredMatchSiloMetadataPlacementFilterStrategy = (filterStrategy as PreferredMatchSiloMetadataPlacementFilterStrategy);
17+
var preferredMatchSiloMetadataPlacementFilterStrategy = filterStrategy as PreferredMatchSiloMetadataPlacementFilterStrategy;
1818
var minCandidates = preferredMatchSiloMetadataPlacementFilterStrategy?.MinCandidates ?? 1;
1919
var orderedMetadataKeys = preferredMatchSiloMetadataPlacementFilterStrategy?.OrderedMetadataKeys ?? [];
2020

21-
var localSiloMetadata = siloMetadataCache.GetMetadata(localSiloDetails.SiloAddress).Metadata;
21+
var localSiloMetadata = siloMetadataCache.GetSiloMetadata(localSiloDetails.SiloAddress).Metadata;
2222

2323
if (localSiloMetadata.Count == 0)
2424
{
@@ -39,7 +39,7 @@ public IEnumerable<SiloAddress> Filter(PlacementFilterStrategy filterStrategy, P
3939
var scoreCounts = new int[orderedMetadataKeys.Length+1];
4040
for (var i = 0; i < siloList.Count; i++)
4141
{
42-
var siloMetadata = siloMetadataCache.GetMetadata(siloList[i]).Metadata;
42+
var siloMetadata = siloMetadataCache.GetSiloMetadata(siloList[i]).Metadata;
4343
var siloScore = 0;
4444
for (var j = orderedMetadataKeys.Length - 1; j >= 0; --j)
4545
{

‎src/Orleans.Runtime/Placement/Filtering/PreferredMatchSiloMetadataPlacementFilterStrategy.cs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Globalization;
34
using Orleans.Metadata;
45
using Orleans.Placement;
56

@@ -23,6 +24,7 @@ public override void AdditionalInitialize(GrainProperties properties)
2324
{
2425
throw new ArgumentException("Invalid ordered-metadata-keys property value.");
2526
}
27+
2628
OrderedMetadataKeys = placementFilterGrainProperty.Split(",");
2729
var minCandidatesProperty = GetPlacementFilterGrainProperty("min-candidates", properties);
2830
if (!int.TryParse(minCandidatesProperty, out var parsedMinCandidates))
@@ -37,6 +39,6 @@ protected override IEnumerable<KeyValuePair<string, string>> GetAdditionalGrainP
3739
IReadOnlyDictionary<string, string> existingProperties)
3840
{
3941
yield return new KeyValuePair<string, string>("ordered-metadata-keys", string.Join(",", OrderedMetadataKeys));
40-
yield return new KeyValuePair<string, string>("min-candidates", MinCandidates.ToString());
42+
yield return new KeyValuePair<string, string>("min-candidates", MinCandidates.ToString(CultureInfo.InvariantCulture));
4143
}
4244
}

0 commit comments

Comments
 (0)