Align all membership providers' CleanupDefunctSiloEntries to remove non-Active stale entries

[Copilot]

• Fix InMemoryMembershipTable.CleanupDefunctSiloEntries to remove all non-Active old entries (not just Dead entries)
• Fix InProcessMembershipTable.CleanupDefunctSiloEntries to use the same != SiloStatus.Active predicate for consistency
• Add unit tests for the development clustering CleanupDefunctSiloEntries in InMemoryMembershipTableTests
• Fix CosmosMembershipTable.CleanupDefunctSiloEntries — push Status != Active filter server-side (indexed), then apply date check in C# using Math.Max(IAmAliveTime.Ticks, StartTime.Ticks) < beforeDate.Ticks to preserve correct semantics when IAmAliveTime is uninitialized
• Implement ZooKeeperBasedMembershipTable.CleanupDefunctSiloEntries — was NotImplementedException; deletes non-Active entries using Math.Max(IAmAliveTime.Ticks, StartTime.Ticks) < beforeDate.Ticks date comparison (ZooKeeper has no query capability so full materialization is unavoidable)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Implement CleanupDefunctSiloEntries for development clustering</issue_title>
<issue_description>After about 10 mins of inactivity my code is always trying to execute the following code and throws the error

public Task CleanupDefunctSiloEntries(DateTimeOffset beforeDate)
        {
                throw new NotImplementedException();
        }

in SystemTargetBasedMembershipTable.cs

What is the reason of this?

My startup code

builder.Host.UseOrleans(silo =>
{
    silo
        .UseLocalhostClustering()
        .ConfigureLogging(options => options.SetMinimumLevel(LogLevel.Information))
        .AddIncomingGrainCallFilter<LoggingInterceptor>()
        .AddAdoNetGrainStorage(StateStorageConstants.StorageName, options =>
        {
            options.Invariant = "Npgsql";
            options.ConnectionString = builder.Configuration.GetConnectionString("postgres");
        });
});

</issue_description>

<agent_instructions>do a good job</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@ReubenBond That error is benign. The "localhost clustering" is only intended for dev/test, so generally doesn't live long-enough to warrant cleaning up defunct silos.

Regardless, we should implement it.</comment_new>
<comment_new>@
We've moved this issue to the Backlog. This means that it is not going to be worked on for the coming release. We review items in the backlog at the end of each milestone/release and depending on the team's priority we may reconsider this issue for the following milestone. </comment_new>

• Fixes Implement CleanupDefunctSiloEntries for development clustering #8256

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Microsoft Reviewers: Open in CodeFlow

[ReubenBond]

@copilot note that we have an implementation of an in-memory membership table cleanup here: src\Orleans.TestingHost\InProcess\InProcessMembershipTable.cs. You can take inspiration from that.

[Copilot]

@copilot note that we have an implementation of an in-memory membership table cleanup here: src\Orleans.TestingHost\InProcess\InProcessMembershipTable.cs. You can take inspiration from that.

Fixed InProcessMembershipTable.CleanupDefunctSiloEntries in commit 3d056d3 to apply the same != SiloStatus.Active predicate, keeping it consistent with InMemoryMembershipTable and all other provider implementations.

[rkargMsft]

@copilot the issue #8256 appears to have already been addressed but we should do alignment of all membership providers to remove non-Active silos in CleanupDefunctSiloEntries. Update this PR to be that alignment across all providers.

[Copilot]

@copilot the issue #8256 appears to have already been addressed but we should do alignment of all membership providers to remove non-Active si...

Done in commit 493f3b1. Updated all providers to remove non-Active stale entries:

• ZooKeeper — implemented CleanupDefunctSiloEntries (was NotImplementedException): reads all entries, deletes any with status != Active older than the cutoff
• Cosmos — changed from ReadSilos(SiloStatus.Dead) to ReadSilos() with (SiloStatus)s.Status != SiloStatus.Active filter; also aligned the timestamp check to use Math.Max(IAmAliveTime, StartTime) like all other providers

The other providers (Redis, Cassandra, AzureStorage, Consul, DynamoDB, AdoNet) were already using the correct non-Active predicate.

[rkargMsft]

@copilot do the new Cosmos and Zookeeper implementations materialize the whole silo list and then filter down to not active silos? Is there an elegant way to do filtering on the database side without needing to make any changes to the database schema/layout/indexes/etc.?

[Copilot]

@copilot do the new Cosmos and Zookeeper implementations materialize the whole silo list and then filter down to not active silos? Is there an elegant way to do filtering on the data...

Cosmos (commit e03465f): Yes, the previous version called ReadSilos() with no args (fetches all silos server-side), then filtered in memory. It's now updated to build the LINQ query with all predicates pushed to the server — Status != Active uses the existing index, and IAmAliveTime < beforeDate && StartTime < beforeDate (equivalent to Math.Max(IAmAliveTime, StartTime) < beforeDate) are also server-side predicates. StartTime/IAmAliveTime are excluded from the Cosmos index, but the Status filter reduces the result set without any schema changes.

ZooKeeper: ZooKeeper is a hierarchical KV store with no query capability — you list child node names, then fetch each node's data individually. There's no way to push predicates to the server. Full materialization is inherent to its architecture, so the current implementation is already optimal.

[rkargMsft]

@copilot I don't think that strictly IAmAliveTime < beforeDate && StartTime < beforeDate in Cosmos is equivalent to Math.Max(IAmAliveTime, StartTime) < beforeDate) in the C# code. If IAmAliveTime doesn't have a value assigned then it will not get returned by the query, but functionally what we want is to look for an older IAmAliveTime older than beforeDate but if that's not defined (C# would map that to DateTime.MinValue) then check that StartTime is older than beforeDate.

If the above is correct then let's go with doing just the Status filtering in Cosmos (since that's indexed already) and then keep the date filtering all in C# to preserve the desired behavior.

[Copilot]

@copilot I don't think that strictly IAmAliveTime < beforeDate && StartTime < beforeDate in Cosmos is equivalent to Math.Max(IAmAliveTime, StartTime) < beforeDate) in the C# code...

Fixed in commit 2d0523f. The Cosmos query now only pushes Status != Active to the server (which uses the existing index), then applies the Math.Max(IAmAliveTime, StartTime) < beforeDate date check in C# to preserve the correct behavior when IAmAliveTime is uninitialized.

[rkargMsft]

@copilot instead of creating a new datetime here, compare the max ticks to the beforeDate.Ticks.

Make the same update to the Cosmos implementation.

[Copilot]

Done in commit 0d6d77c. Both ZooKeeper and Cosmos now use Math.Max(...Ticks) < beforeDate.Ticks directly.