Skip to content

Commit 044385f

Browse files
ReubenBondCopilot
andcommitted
refactor: simplify cluster manifest local silo handling
Remove synchronous local silo insertion from the cluster manifest provider and let the standard manifest fetch path populate all active silos. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8a535fc commit 044385f

2 files changed

Lines changed: 15 additions & 34 deletions

File tree

src/Orleans.Runtime/Manifest/ClusterManifestProvider.cs

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ namespace Orleans.Runtime.Metadata
1616
{
1717
internal partial class ClusterManifestProvider : IClusterManifestProvider, IAsyncDisposable, IDisposable, ILifecycleParticipant<ISiloLifecycle>
1818
{
19-
private readonly SiloAddress _localSiloAddress;
2019
private readonly ILogger<ClusterManifestProvider> _logger;
2120
private readonly IServiceProvider _services;
2221
private readonly IClusterMembershipService _clusterMembershipService;
@@ -33,14 +32,12 @@ internal partial class ClusterManifestProvider : IClusterManifestProvider, IAsyn
3332
private Task? _runTask;
3433

3534
public ClusterManifestProvider(
36-
ILocalSiloDetails localSiloDetails,
3735
SiloManifestProvider siloManifestProvider,
3836
IClusterMembershipService clusterMembershipService,
3937
IFatalErrorHandler fatalErrorHandler,
4038
ILogger<ClusterManifestProvider> logger,
4139
IServiceProvider services)
4240
{
43-
_localSiloAddress = localSiloDetails.SiloAddress;
4441
_logger = logger;
4542
_services = services;
4643
_clusterMembershipService = clusterMembershipService;
@@ -70,7 +67,7 @@ private ClusterManifest EnsureCurrentManifestVersion(ClusterMembershipSnapshot c
7067
return current;
7168
}
7269

73-
var synchronizedSilos = ApplySynchronousMembershipChanges(current.Silos, clusterMembership, out var modified);
70+
var synchronizedSilos = RemoveNonActiveSilos(current.Silos, clusterMembership, out var modified);
7471
if (current.Version.Major == membershipVersion && !modified)
7572
{
7673
return current;
@@ -84,7 +81,7 @@ private ClusterManifest EnsureCurrentManifestVersion(ClusterMembershipSnapshot c
8481
return current;
8582
}
8683

87-
synchronizedSilos = ApplySynchronousMembershipChanges(current.Silos, clusterMembership, out modified);
84+
synchronizedSilos = RemoveNonActiveSilos(current.Silos, clusterMembership, out modified);
8885
if (current.Version.Major == membershipVersion && !modified)
8986
{
9087
return current;
@@ -190,20 +187,14 @@ private async Task<bool> UpdateManifest(ClusterMembershipSnapshot clusterMembers
190187
continue;
191188
}
192189

193-
if (siloAddress.Equals(_localSiloAddress))
194-
{
195-
// Local membership changes are applied synchronously by EnsureCurrentManifestVersion.
196-
continue;
197-
}
198-
199190
tasks.Add(GetManifest(siloAddress));
200191
}
201192

202193
async Task<(SiloAddress Key, GrainManifest? Value, Exception? Exception)> GetManifest(SiloAddress siloAddress)
203194
{
204195
try
205196
{
206-
// Get the manifest from the remote silo.
197+
// Get the manifest from the silo.
207198
var remoteManifestProvider = _grainFactory!.GetSystemTarget<ISiloManifestSystemTarget>(Constants.ManifestProviderType, siloAddress);
208199
var manifest = await remoteManifestProvider.GetSiloManifest().AsTask().WaitAsync(_shutdownCts.Token);
209200
return (siloAddress, manifest, null);
@@ -253,7 +244,7 @@ private ClusterManifest CreateClusterManifest(
253244
MajorMinorVersion version,
254245
ImmutableDictionary<SiloAddress, GrainManifest> silos)
255246
{
256-
return new ClusterManifest(version, silos, [.. silos.Values, LocalGrainManifest]);
247+
return new ClusterManifest(version, silos, [LocalGrainManifest]);
257248
}
258249

259250
private bool TryPublishManifest(ClusterManifest manifest)
@@ -267,21 +258,6 @@ private bool TryPublishManifest(ClusterManifest manifest)
267258
return publishSuccess;
268259
}
269260

270-
private ImmutableDictionary<SiloAddress, GrainManifest> ApplySynchronousMembershipChanges(
271-
ImmutableDictionary<SiloAddress, GrainManifest> silos,
272-
ClusterMembershipSnapshot clusterMembership,
273-
out bool modified)
274-
{
275-
silos = RemoveNonActiveSilos(silos, clusterMembership, out modified);
276-
if (clusterMembership.GetSiloStatus(_localSiloAddress) != SiloStatus.Active || silos.ContainsKey(_localSiloAddress))
277-
{
278-
return silos;
279-
}
280-
281-
modified = true;
282-
return silos.Add(_localSiloAddress, LocalGrainManifest);
283-
}
284-
285261
private static ImmutableDictionary<SiloAddress, GrainManifest> RemoveNonActiveSilos(
286262
ImmutableDictionary<SiloAddress, GrainManifest> silos,
287263
ClusterMembershipSnapshot clusterMembership,

test/Orleans.Core.Tests/Manifest/ClusterManifestProviderTests.cs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,11 @@ public async Task Current_WhenRemoteSiloBecomesActive_PublishesPrunedManifestBef
103103
var grainFactory = CreateGrainFactory(remoteSilo, remoteManifest);
104104
var provider = CreateClusterManifestProvider(localSilo, membership, grainFactory);
105105

106-
Assert.Equal(new MajorMinorVersion(1, 0), provider.Current.Version);
107-
Assert.DoesNotContain(remoteSilo, provider.Current.Silos.Keys);
106+
var current = provider.Current;
107+
Assert.Equal(new MajorMinorVersion(1, 0), current.Version);
108+
Assert.DoesNotContain(localSilo, current.Silos.Keys);
109+
Assert.DoesNotContain(remoteSilo, current.Silos.Keys);
110+
Assert.Contains(provider.LocalGrainManifest, current.AllGrainManifests);
108111

109112
membership.Update(CreateMembershipSnapshot(
110113
2,
@@ -113,6 +116,7 @@ public async Task Current_WhenRemoteSiloBecomesActive_PublishesPrunedManifestBef
113116

114117
var pruned = provider.Current;
115118
Assert.Equal(new MajorMinorVersion(2, 0), pruned.Version);
119+
Assert.DoesNotContain(localSilo, pruned.Silos.Keys);
116120
Assert.DoesNotContain(remoteSilo, pruned.Silos.Keys);
117121

118122
var lifecycle = await StartAsync(provider);
@@ -170,16 +174,17 @@ private static ClusterManifestProvider CreateClusterManifestProvider(
170174
TestClusterMembershipService membership,
171175
IInternalGrainFactory grainFactory)
172176
{
173-
var localSiloDetails = Substitute.For<ILocalSiloDetails>();
174-
localSiloDetails.SiloAddress.Returns(localSilo);
177+
var siloManifestProvider = CreateSiloManifestProvider();
178+
grainFactory
179+
.GetSystemTarget<ISiloManifestSystemTarget>(Constants.ManifestProviderType, localSilo)
180+
.Returns(new TestSiloManifestSystemTarget(siloManifestProvider.SiloManifest));
175181

176182
var services = new ServiceCollection()
177183
.AddSingleton(grainFactory)
178184
.BuildServiceProvider();
179185

180186
return new ClusterManifestProvider(
181-
localSiloDetails,
182-
CreateSiloManifestProvider(),
187+
siloManifestProvider,
183188
membership,
184189
Substitute.For<IFatalErrorHandler>(),
185190
NullLogger<ClusterManifestProvider>.Instance,

0 commit comments

Comments
 (0)