From 3d37046d2304804bba91646d4840fcbf62c39c36 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 19:54:23 -0700 Subject: [PATCH 01/21] implement subscribe to entry state change, pivot to setter properties --- .../Models/JobRepositoryEntry.cs | 76 +++++++++++-------- .../Heartbeats/HeartbeatMaintainer.cs | 6 +- .../Services/Jobs/JobExecutor.cs | 4 +- .../Services/Jobs/JobRepository.cs | 6 +- .../Tests/JobResultTranslationTests.cs | 4 +- .../Tests/Models/JobRepositoryEntryTests.cs | 69 +++++++++++++---- .../Services/Heartbeats/MaintainerTests.cs | 76 ++++++++----------- .../Tests/Services/Jobs/JobExecutorTests.cs | 28 +++---- .../Tests/Services/Jobs/JobRepositoryTests.cs | 19 ++--- 9 files changed, 158 insertions(+), 130 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index fd797432..cb04d2ef 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -11,13 +11,17 @@ internal interface ISortableJobWrapper internal interface IJobRepositoryEntry : ISortableJobWrapper { IRawJobModel RawJobModel { get; } - bool CanHeartbeat { get; } - DateTime LastHeartbeatTime { get; } - JobState State { get; } - Task SetAsCannotHeartbeatAsync(CancellationToken cancellationToken = default); - Task SetLastHeartbeatTimeAsync(DateTime lastHeartbeatTime, CancellationToken cancellationToken = default); - Task SetStateAsync(JobState state, CancellationToken cancellationToken = default); + /// + /// Whether this job can still receive heartbeats. + /// May only be set to false. + /// + bool CanHeartbeat { get; set; } + + DateTime LastHeartbeatTime { get; set; } + JobState State { get; set; } + + void SubscribeToStateChange(Action action); } internal sealed class JobRepositoryEntry : IJobRepositoryEntry @@ -30,6 +34,7 @@ internal sealed class JobRepositoryEntry : IJobRepositoryEntry private bool _canHeartbeat = true; private DateTime _lastHeartbeatTime; private JobState _state = JobState.Inactive; + private Action? _stateChangeCallbacks; public required IRawJobModel RawJobModel { get; init; } public required IJobModel JobModel { get; init; } @@ -43,6 +48,18 @@ public bool CanHeartbeat return _canHeartbeat; } } + set + { + if (value) + { + throw new ArgumentException("CanHeartbeat can only be set to false.", nameof(value)); + } + + lock (_lock) + { + _canHeartbeat = false; + } + } } public DateTime LastHeartbeatTime @@ -54,6 +71,13 @@ public DateTime LastHeartbeatTime return _lastHeartbeatTime; } } + set + { + lock (_lock) + { + _lastHeartbeatTime = value; + } + } } public JobState State @@ -65,39 +89,31 @@ public JobState State return _state; } } - } - - public Task SetAsCannotHeartbeatAsync(CancellationToken cancellationToken = default) - { - cancellationToken.ThrowIfCancellationRequested(); - lock (_lock) + set { - _canHeartbeat = false; - } + Action? callbacks; + lock (_lock) + { + if (_state == value) + { + return; + } - return Task.CompletedTask; - } + _state = value; + callbacks = _stateChangeCallbacks; + } - public Task SetLastHeartbeatTimeAsync(DateTime lastHeartbeatTime, - CancellationToken cancellationToken = default) - { - cancellationToken.ThrowIfCancellationRequested(); - lock (_lock) - { - _lastHeartbeatTime = lastHeartbeatTime; + callbacks?.Invoke(value); } - - return Task.CompletedTask; } - public Task SetStateAsync(JobState state, CancellationToken cancellationToken = default) + public void SubscribeToStateChange(Action action) { - cancellationToken.ThrowIfCancellationRequested(); + ArgumentNullException.ThrowIfNull(action); + lock (_lock) { - _state = state; + _stateChangeCallbacks += action; } - - return Task.CompletedTask; } } \ No newline at end of file diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs index f06cb14f..80c4f2fb 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs @@ -114,7 +114,7 @@ await sleepService.DelayAsync(TimeSpan.FromSeconds(Math.Pow(2, args.AttemptNumbe await GetRetryPipeline().ExecuteAsync( async token => await jobSource.HeartbeatAsync(jobRepositoryEntry.RawJobModel, token), cancellationToken); - await jobRepositoryEntry.SetLastHeartbeatTimeAsync(DateTime.UtcNow, cancellationToken); + jobRepositoryEntry.LastHeartbeatTime = DateTime.UtcNow; } catch (WorkerJobSourceException e) { @@ -124,7 +124,7 @@ await GetRetryPipeline().ExecuteAsync( // by the time the next loop iteration comes around. // // The documented recommendation for a heartbeat interval is ~75% of the time until message expiry - await jobRepositoryEntry.SetAsCannotHeartbeatAsync(cancellationToken); + jobRepositoryEntry.CanHeartbeat = false; } catch (Exception ex) when (ex is not OperationCanceledException) { @@ -136,7 +136,7 @@ await GetRetryPipeline().ExecuteAsync( throw; } - await jobRepositoryEntry.SetAsCannotHeartbeatAsync(cancellationToken); + jobRepositoryEntry.CanHeartbeat = false; } return heartbeatCalculator.TimeUntilNextHeartbeat(jobRepositoryEntry); diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs index 066cb38b..d93574a2 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs @@ -127,7 +127,7 @@ public async Task RunAsync(int executorId, Cancellatio logger.LogTrace( "Executor {Id} was unable to obtain a lock on message {MessageId} , deferring to Idempotency Monitor", executorId, repositoryEntry.JobModel.MessageId); - await repositoryEntry.SetStateAsync(JobState.BlockedByIdempotency, cancellationToken); + repositoryEntry.State = JobState.BlockedByIdempotency; continue; } @@ -136,7 +136,7 @@ public async Task RunAsync(int executorId, Cancellatio // Mark as complete for all branches of ActOnJobAsync by doing it afterwards // Reminder that JobState does not imply anything about success or acknowledgement success. // It only means that the JobWorker is done with the job. - await repositoryEntry.SetStateAsync(JobState.Complete, cancellationToken); + repositoryEntry.State = JobState.Complete; await jobRepository.RemoveJobAsync(repositoryEntry, cancellationToken); } finally diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index 825c3f39..723354fb 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -288,7 +288,7 @@ public async Task GetWatchedJobsCountAsync(CancellationToken cancellationTo await _jobsAvailableEvent.WaitAsync(TimeSpan.FromMilliseconds(250), cancellationToken); } while (result is null); - await result.SetStateAsync(JobState.Active, cancellationToken); + result.State = JobState.Active; NotifyInactiveCountUpdate(await GetInactiveJobCountAsync(cancellationToken)); return result; @@ -319,7 +319,7 @@ public async Task LoadAsync(IReadOnlyList intakeItems, JobModel = envelope.JobModel, RawJobModel = envelope.RawJobModel }; - await job.SetLastHeartbeatTimeAsync(DateTime.UtcNow, cancellationToken); + job.LastHeartbeatTime = DateTime.UtcNow; _inactiveJobsList.Add(job); // Worry about sorting later, see below @@ -357,7 +357,7 @@ public async Task LoadAsync(IReadOnlyList intakeItems, public async Task ReloadUnblockedJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default) { - await job.SetStateAsync(JobState.Inactive, cancellationToken); + job.State = JobState.Inactive; // Shortlist the job for re-execution in memory _unblockedJobsQueue.Enqueue(job); // Tell any active invocations of GetNextJobAsync that there is something available. diff --git a/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs b/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs index ef89f651..a86696ee 100644 --- a/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs @@ -64,9 +64,7 @@ public async Task JobResult_FromLogicRunner_IsTranslated_ToJobSource_AndFailureH var repositoryEntry = new Mock(MockBehavior.Strict); repositoryEntry.Setup(e => e.JobModel).Returns(job.Object); repositoryEntry.Setup(e => e.RawJobModel).Returns(rawJob.Object); - repositoryEntry - .Setup(e => e.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + repositoryEntry.SetupSet(e => e.State = JobState.Complete); // Application logic var logicRunner = new Mock(MockBehavior.Strict); diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 584bc419..9ebd1d50 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -6,28 +6,42 @@ namespace RedShirt.Example.JobWorker.Core.UnitTests.Tests.Models; public class JobRepositoryEntryTests { - [Fact] - public async Task ConcurrentReadsAndWrites_DoNotThrow() + private static JobRepositoryEntry CreateEntry() { - var jre = new JobRepositoryEntry + return new JobRepositoryEntry { JobModel = new Mock(MockBehavior.Strict).Object, RawJobModel = new Mock(MockBehavior.Strict).Object }; + } + + [Fact] + public void CanHeartbeat_WhenSetTrue_ThrowsArgumentException() + { + var jre = CreateEntry(); + + var ex = Assert.Throws(() => jre.CanHeartbeat = true); + Assert.Equal("value", ex.ParamName); + Assert.True(jre.CanHeartbeat); + } + + [Fact] + public async Task ConcurrentReadsAndWrites_DoNotThrow() + { + var jre = CreateEntry(); - var writers = Enumerable.Range(0, 8).Select(async i => + var writers = Enumerable.Range(0, 8).Select(i => Task.Run(() => { for (var n = 0; n < 100; n++) { - await jre.SetStateAsync((JobState) (n % 4), TestContext.Current.CancellationToken); - await jre.SetLastHeartbeatTimeAsync(DateTime.UtcNow.AddSeconds(-n), - TestContext.Current.CancellationToken); + jre.State = (JobState) (n % 4); + jre.LastHeartbeatTime = DateTime.UtcNow.AddSeconds(-n); if (i == 0 && n == 50) { - await jre.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken); + jre.CanHeartbeat = false; } } - }); + }, TestContext.Current.CancellationToken)); var readers = Enumerable.Range(0, 8).Select(_ => Task.Run(() => { @@ -44,7 +58,34 @@ await jre.SetLastHeartbeatTimeAsync(DateTime.UtcNow.AddSeconds(-n), } [Fact] - public async Task TestGettersSetters() + public void SubscribeToStateChange_WhenNull_ThrowsArgumentNullException() + { + var jre = CreateEntry(); + + Assert.Throws(() => jre.SubscribeToStateChange(null!)); + } + + [Fact] + public void SubscribeToStateChange_WhenStateChanges_InvokesCallbacks() + { + var jre = CreateEntry(); + var first = new List(); + var second = new List(); + + jre.SubscribeToStateChange(first.Add); + jre.SubscribeToStateChange(second.Add); + + jre.State = JobState.Active; + jre.State = JobState.Active; + jre.State = JobState.Complete; + + Assert.Equal([JobState.Active, JobState.Complete], first); + Assert.Equal([JobState.Active, JobState.Complete], second); + Assert.Equal(JobState.Complete, jre.State); + } + + [Fact] + public void TestGettersSetters() { var jobModel = new Mock(MockBehavior.Strict).Object; var rawJobModel = new Mock(MockBehavior.Strict).Object; @@ -61,15 +102,15 @@ public async Task TestGettersSetters() // Set/Get Heartbeat Time Assert.Equal(default, jre.LastHeartbeatTime); var newDate = DateTime.UtcNow - TimeSpan.FromMinutes(2); - await jre.SetLastHeartbeatTimeAsync(newDate, TestContext.Current.CancellationToken); + jre.LastHeartbeatTime = newDate; Assert.Equal(newDate, jre.LastHeartbeatTime); // Set/Get State - await jre.SetStateAsync(JobState.Active, TestContext.Current.CancellationToken); + jre.State = JobState.Active; Assert.Equal(JobState.Active, jre.State); - // Set/Get FlightTimeCanBeExtended - await jre.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken); + // Set/Get CanHeartbeat (false only) + jre.CanHeartbeat = false; Assert.False(jre.CanHeartbeat); } } \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs index bd2d3fcf..79d85dd6 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs @@ -75,8 +75,7 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailureFals entry.Setup(e => e.JobModel).Returns(subject.Object); entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); entry.Setup(e => e.State).Returns(JobState.Active); - entry.Setup(e => e.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.CanHeartbeat = false); var heartbeatCalculator = new Mock(); heartbeatCalculator.Setup(c => c.IsReadyForHeartbeat(entry.Object)).Returns(true); @@ -124,7 +123,7 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailureFals await maintainer.RunAsync(TestContext.Current.CancellationToken); health.Verify(h => h.NoteIncident(), Times.Once); - entry.Verify(e => e.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken), Times.Once); + entry.VerifySet(e => e.CanHeartbeat = false, Times.Once); } [Fact(Timeout = 1500)] @@ -176,7 +175,7 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailure_Pro Assert.Same(unexpected, thrown); health.Verify(h => h.NoteIncident(), Times.Once); - entry.Verify(e => e.SetAsCannotHeartbeatAsync(It.IsAny()), Times.Never); + entry.VerifySet(e => e.CanHeartbeat = false, Times.Never); } /// @@ -194,10 +193,9 @@ public async Task TestFilterOutCannotHeartbeatJobs() entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); entry.Setup(e => e.CanHeartbeat).Returns(true); entry.Setup(e => e.State).Returns(JobState.Active); - entry.Setup(e => e.SetLastHeartbeatTimeAsync(It.Is(dt => - dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && - dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250)), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.LastHeartbeatTime = It.Is(dt => + dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && + dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250))); var redHerringEntry = new Mock(MockBehavior.Strict); redHerringEntry.Setup(e => e.State).Returns(JobState.Active); redHerringEntry.Setup(e => e.CanHeartbeat).Returns(false); @@ -317,10 +315,9 @@ public async Task TestHeartbeatSingleJob() entry.Setup(e => e.JobModel).Returns(subject.Object); entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); entry.Setup(e => e.State).Returns(JobState.Active); - entry.Setup(e => e.SetLastHeartbeatTimeAsync(It.Is(dt => - dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && - dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250)), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.LastHeartbeatTime = It.Is(dt => + dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && + dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250))); var heartbeatCalculator = new Mock(); heartbeatCalculator @@ -388,13 +385,11 @@ public async Task TestHeartbeatSingleJobButGotHeartbeatException() var rawJobModel = new Mock(MockBehavior.Strict); entry.Setup(e => e.JobModel).Returns(subject.Object); entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); - entry.Setup(e => e.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.CanHeartbeat = false); entry.Setup(e => e.State).Returns(JobState.Active); - entry.Setup(e => e.SetLastHeartbeatTimeAsync(It.Is(dt => - dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && - dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250)), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.LastHeartbeatTime = It.Is(dt => + dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && + dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250))); var heartbeatCalculator = new Mock(); heartbeatCalculator @@ -448,8 +443,7 @@ public async Task TestHeartbeatSingleJobButGotHeartbeatException() jobSource.Verify(s => s.HeartbeatAsync(rawJobModel.Object, TestContext.Current.CancellationToken), Times.Once); - entry.Verify(e => e.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken), - Times.Once); + entry.VerifySet(e => e.CanHeartbeat = false, Times.Once); } /// @@ -528,8 +522,7 @@ public async Task TestHeartbeatSingleJob_ExhaustsTransientRetriesThenDisablesExt var rawJobModel = new Mock(MockBehavior.Strict); entry.Setup(e => e.JobModel).Returns(subject.Object); entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); - entry.Setup(e => e.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.CanHeartbeat = false); entry.Setup(e => e.State).Returns(JobState.Active); var heartbeatCalculator = new Mock(MockBehavior.Strict); @@ -579,8 +572,7 @@ public async Task TestHeartbeatSingleJob_ExhaustsTransientRetriesThenDisablesExt jobSource.Verify(s => s.HeartbeatAsync(rawJobModel.Object, TestContext.Current.CancellationToken), Times.Exactly(Globals.HeartbeatRetryCount + 1)); - entry.Verify(e => e.SetAsCannotHeartbeatAsync(TestContext.Current.CancellationToken), - Times.Once); + entry.VerifySet(e => e.CanHeartbeat = false, Times.Once); } /// @@ -597,10 +589,9 @@ public async Task TestHeartbeatSingleJob_NotReadyYet() entry.Setup(e => e.JobModel).Returns(subject.Object); entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); entry.Setup(e => e.State).Returns(JobState.Active); - entry.Setup(e => e.SetLastHeartbeatTimeAsync(It.Is(dt => - dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && - dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250)), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.LastHeartbeatTime = It.Is(dt => + dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && + dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250))); var heartbeatCalculator = new Mock(); heartbeatCalculator @@ -666,10 +657,9 @@ public async Task TestHeartbeatSingleJob_PreciseTiming() entry.Setup(e => e.JobModel).Returns(subject.Object); entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); entry.Setup(e => e.State).Returns(JobState.Active); - entry.Setup(e => e.SetLastHeartbeatTimeAsync(It.Is(dt => - dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && - dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250)), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.LastHeartbeatTime = It.Is(dt => + dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && + dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250))); var heartbeatCalculator = new Mock(); heartbeatCalculator @@ -734,8 +724,7 @@ public async Task TestHeartbeatSingleJob_RetriesTransientFailuresThenSucceeds() entry.Setup(e => e.JobModel).Returns(subject.Object); entry.Setup(e => e.RawJobModel).Returns(rawJobModel.Object); entry.Setup(e => e.State).Returns(JobState.Active); - entry.Setup(e => e.SetLastHeartbeatTimeAsync(It.IsAny(), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.LastHeartbeatTime = It.IsAny()); var heartbeatCalculator = new Mock(MockBehavior.Strict); heartbeatCalculator.Setup(c => c.IsReadyForHeartbeat(entry.Object)).Returns(true); @@ -795,9 +784,8 @@ public async Task TestHeartbeatSingleJob_RetriesTransientFailuresThenSucceeds() await maintainer.RunAsync(TestContext.Current.CancellationToken); Assert.Equal(3, attempts); - entry.Verify(e => e.SetAsCannotHeartbeatAsync(It.IsAny()), - Times.Never); - entry.Verify(e => e.SetLastHeartbeatTimeAsync(It.IsAny(), It.IsAny()), Times.Once); + entry.VerifySet(e => e.CanHeartbeat = false, Times.Never); + entry.VerifySet(e => e.LastHeartbeatTime = It.IsAny(), Times.Once); } /// @@ -816,10 +804,9 @@ public async Task TestHeartbeatTwoJob() entry1.Setup(e => e.RawJobModel).Returns(rawJobModel1.Object); entry1.Setup(e => e.CanHeartbeat).Returns(true); entry1.Setup(e => e.State).Returns(JobState.Active); - entry1.Setup(e => e.SetLastHeartbeatTimeAsync(It.Is(dt => - dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && - dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250)), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry1.SetupSet(e => e.LastHeartbeatTime = It.Is(dt => + dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && + dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250))); // Second job var subject2 = new Mock(MockBehavior.Strict); @@ -830,10 +817,9 @@ public async Task TestHeartbeatTwoJob() entry2.Setup(e => e.RawJobModel).Returns(rawJobModel2.Object); entry2.Setup(e => e.State).Returns(JobState.Active); entry2.Setup(e => e.CanHeartbeat).Returns(true); - entry2.Setup(e => e.SetLastHeartbeatTimeAsync(It.Is(dt => - dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && - dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250)), TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry2.SetupSet(e => e.LastHeartbeatTime = It.Is(dt => + dt > DateTime.UtcNow - TimeSpan.FromMilliseconds(250) && + dt < DateTime.UtcNow + TimeSpan.FromMilliseconds(250))); // Heartbeat var heartbeatCalculator = new Mock(); diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs index 4a831375..9038c6a0 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs @@ -52,8 +52,7 @@ public async Task ExecuteSingleJob(CoreJobResult safeRunnerResult) AcknowledgedSuccessfully = true, LoggedFailureSuccessfully = null }; - jobRepositoryEntry.Setup(j => j.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + jobRepositoryEntry.SetupSet(j => j.State = JobState.Complete); var runException = safeRunnerResult == CoreJobResult.Success ? null @@ -121,8 +120,7 @@ public async Task ExecuteSingleJob(CoreJobResult safeRunnerResult) safeAcknowledgementService.Verify( s => s.AcknowledgeSafelyAsync(rawJobModel.Object, safeRunnerResult, runException, null, TestContext.Current.CancellationToken), Times.Once); - jobRepositoryEntry.Verify(j => j.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken), - Times.Once); + jobRepositoryEntry.VerifySet(j => j.State = JobState.Complete, Times.Once); idempotencyExecutionService.Verify( s => s.SetResultInCacheAsync(rawJobModel.Object, safeRunnerResult, ackResult, TestContext.Current.CancellationToken), Times.Once); @@ -168,8 +166,7 @@ public async Task PrepareToExitOnNull() public async Task WhenCachedResultIsSuccessAndAcknowledgeFails_SkipsExecution() { var (jobRepositoryEntry, jobModel, rawJobModel) = CreateRepositoryEntry(); - jobRepositoryEntry.Setup(j => j.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + jobRepositoryEntry.SetupSet(j => j.State = JobState.Complete); var cachedResult = new IdempotencyCacheResult { JobResult = CoreJobResult.Success, @@ -230,8 +227,7 @@ public async Task WhenCachedResultIsSuccessAndAcknowledgeFails_SkipsExecution() s => s.SetResultInCacheAsync(rawJobModel.Object, CoreJobResult.Success, failedAck, TestContext.Current.CancellationToken), Times.Once); - jobRepositoryEntry.Verify(j => j.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken), - Times.Once); + jobRepositoryEntry.VerifySet(j => j.State = JobState.Complete, Times.Once); jobRepository.Verify(r => r.RemoveJobAsync(jobRepositoryEntry.Object, TestContext.Current.CancellationToken), Times.Once); } @@ -240,8 +236,7 @@ public async Task WhenCachedResultIsSuccessAndAcknowledgeFails_SkipsExecution() public async Task WhenCachedResultIsSuccessAndAcknowledgeSucceeds_SkipsExecution() { var (jobRepositoryEntry, jobModel, rawJobModel) = CreateRepositoryEntry(); - jobRepositoryEntry.Setup(j => j.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + jobRepositoryEntry.SetupSet(j => j.State = JobState.Complete); var cachedResult = new IdempotencyCacheResult { JobResult = CoreJobResult.Success, @@ -303,8 +298,7 @@ public async Task WhenCachedResultIsSuccessAndAcknowledgeSucceeds_SkipsExecution s => s.SetResultInCacheAsync(rawJobModel.Object, CoreJobResult.Success, successAck, TestContext.Current.CancellationToken), Times.Once); - jobRepositoryEntry.Verify(j => j.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken), - Times.Once); + jobRepositoryEntry.VerifySet(j => j.State = JobState.Complete, Times.Once); jobRepository.Verify(r => r.RemoveJobAsync(jobRepositoryEntry.Object, TestContext.Current.CancellationToken), Times.Once); } @@ -320,8 +314,7 @@ public async Task WhenCachedResultIsUnsuccessful_RunsJobAsRetry(CoreJobResult sa AcknowledgedSuccessfully = true, LoggedFailureSuccessfully = null }; - jobRepositoryEntry.Setup(j => j.SetStateAsync(JobState.Complete, TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + jobRepositoryEntry.SetupSet(j => j.State = JobState.Complete); var runException = safeRunnerResult == CoreJobResult.Success ? null @@ -394,9 +387,7 @@ public async Task WhenCachedResultIsUnsuccessful_RunsJobAsRetry(CoreJobResult sa public async Task WhenIdempotencyLockNotAcquired_MarksJobBlockedAndContinues() { var (jobRepositoryEntry, jobModel, _) = CreateRepositoryEntry(); - jobRepositoryEntry - .Setup(j => j.SetStateAsync(JobState.BlockedByIdempotency, TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + jobRepositoryEntry.SetupSet(j => j.State = JobState.BlockedByIdempotency); var idempotencyLock = new Mock(MockBehavior.Strict); idempotencyLock.SetupGet(l => l.IsAcquired).Returns(false); @@ -425,8 +416,7 @@ public async Task WhenIdempotencyLockNotAcquired_MarksJobBlockedAndContinues() await executor.RunAsync(0, TestContext.Current.CancellationToken); - jobRepositoryEntry.Verify( - j => j.SetStateAsync(JobState.BlockedByIdempotency, TestContext.Current.CancellationToken), Times.Once); + jobRepositoryEntry.VerifySet(j => j.State = JobState.BlockedByIdempotency, Times.Once); idempotencyExecutionService.Verify( s => s.GetCachedResultAsync(It.IsAny(), It.IsAny()), Times.Never); } diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs index 3d0cfcfb..b6d9686f 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs @@ -100,13 +100,11 @@ await jobRepository.LoadAsync( var blockedState = JobState.BlockedByIdempotency; blockedEntry.Setup(e => e.State).Returns(() => blockedState); blockedEntry - .Setup(e => e.SetStateAsync(JobState.Inactive, TestContext.Current.CancellationToken)) - .Callback(() => blockedState = JobState.Inactive) - .Returns(Task.CompletedTask); + .SetupSet(e => e.State = JobState.Inactive) + .Callback(() => blockedState = JobState.Inactive); blockedEntry - .Setup(e => e.SetStateAsync(JobState.Active, TestContext.Current.CancellationToken)) - .Callback(() => blockedState = JobState.Active) - .Returns(Task.CompletedTask); + .SetupSet(e => e.State = JobState.Active) + .Callback(() => blockedState = JobState.Active); jobRepository.WatchedJobs.Add(blockedEntry.Object); await jobRepository.ReloadUnblockedJobAsync(blockedEntry.Object, TestContext.Current.CancellationToken); @@ -114,9 +112,8 @@ await jobRepository.LoadAsync( var nextJob = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); Assert.Same(blockedEntry.Object, nextJob); - blockedEntry.Verify(e => e.SetStateAsync(JobState.Inactive, TestContext.Current.CancellationToken), - Times.Once); - blockedEntry.Verify(e => e.SetStateAsync(JobState.Active, TestContext.Current.CancellationToken), Times.Once); + blockedEntry.VerifySet(e => e.State = JobState.Inactive, Times.Once); + blockedEntry.VerifySet(e => e.State = JobState.Active, Times.Once); } [Fact(Timeout = 2000)] @@ -646,7 +643,7 @@ public async Task TestLoadJobsAndWaitForJob_RequeuedBacklog() Assert.True(gottenJob.CanHeartbeat); // Imitate JobExecutor by marking the task as blocked by idempotency. // Not strictly necessary, but it does imitate the logic of JobExecutor to put the job on the radar of the Idempotency Monitor. - await gottenJob.SetStateAsync(JobState.BlockedByIdempotency, TestContext.Current.CancellationToken); + gottenJob.State = JobState.BlockedByIdempotency; // Reload the unblocked job, imitating the Idempotency Monitor (this puts it into the queue) await jobRepository.ReloadUnblockedJobAsync(gottenJob, TestContext.Current.CancellationToken); @@ -770,7 +767,7 @@ public async Task TestLoadJobsAndWaitForJob_RequeuedBacklogThenEmpty() Assert.True(gottenJob.CanHeartbeat); // Imitate JobExecutor by marking the task as blocked by idempotency. // Not strictly necessary, but it does imitate the logic of JobExecutor to put the job on the radar of the Idempotency Monitor. - await gottenJob.SetStateAsync(JobState.BlockedByIdempotency, TestContext.Current.CancellationToken); + gottenJob.State = JobState.BlockedByIdempotency; // Reload the unblocked job, imitating the Idempotency Monitor (this puts it into the queue) await jobRepository.ReloadUnblockedJobAsync(gottenJob, TestContext.Current.CancellationToken); From e091b7c1d61b58de2ef6831c82347f80dca2786b Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 19:54:59 -0700 Subject: [PATCH 02/21] Follow IDE suggestion to use field keyword on entry properties --- .../Models/JobRepositoryEntry.cs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index cb04d2ef..32949589 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -31,9 +31,6 @@ internal sealed class JobRepositoryEntry : IJobRepositoryEntry /// private readonly Lock _lock = new(); - private bool _canHeartbeat = true; - private DateTime _lastHeartbeatTime; - private JobState _state = JobState.Inactive; private Action? _stateChangeCallbacks; public required IRawJobModel RawJobModel { get; init; } @@ -45,7 +42,7 @@ public bool CanHeartbeat { lock (_lock) { - return _canHeartbeat; + return field; } } set @@ -57,10 +54,10 @@ public bool CanHeartbeat lock (_lock) { - _canHeartbeat = false; + field = false; } } - } + } = true; public DateTime LastHeartbeatTime { @@ -68,14 +65,14 @@ public DateTime LastHeartbeatTime { lock (_lock) { - return _lastHeartbeatTime; + return field; } } set { lock (_lock) { - _lastHeartbeatTime = value; + field = value; } } } @@ -86,7 +83,7 @@ public JobState State { lock (_lock) { - return _state; + return field; } } set @@ -94,18 +91,18 @@ public JobState State Action? callbacks; lock (_lock) { - if (_state == value) + if (field == value) { return; } - _state = value; + field = value; callbacks = _stateChangeCallbacks; } callbacks?.Invoke(value); } - } + } = JobState.Inactive; public void SubscribeToStateChange(Action action) { From 66ef3d3ecc5b467116ec685a4f83474a4e3e2960 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 19:58:26 -0700 Subject: [PATCH 03/21] make state nullable, but not assignable to null --- .../Models/JobRepositoryEntry.cs | 21 ++++++++++++++----- .../Tests/Models/JobRepositoryEntryTests.cs | 10 +++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index 32949589..e42df01b 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -19,7 +19,13 @@ internal interface IJobRepositoryEntry : ISortableJobWrapper bool CanHeartbeat { get; set; } DateTime LastHeartbeatTime { get; set; } - JobState State { get; set; } + + /// + /// Current processing state of this job. + /// May not be set to null. + /// + /// Thrown when the setter is given null. + JobState? State { get; set; } void SubscribeToStateChange(Action action); } @@ -77,7 +83,7 @@ public DateTime LastHeartbeatTime } } - public JobState State + public JobState? State { get { @@ -88,19 +94,24 @@ public JobState State } set { + if (value is not { } newState) + { + throw new ArgumentNullException(nameof(value)); + } + Action? callbacks; lock (_lock) { - if (field == value) + if (field == newState) { return; } - field = value; + field = newState; callbacks = _stateChangeCallbacks; } - callbacks?.Invoke(value); + callbacks?.Invoke(newState); } } = JobState.Inactive; diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 9ebd1d50..17188857 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -57,6 +57,16 @@ public async Task ConcurrentReadsAndWrites_DoNotThrow() Assert.False(jre.CanHeartbeat); } + [Fact] + public void State_WhenSetNull_ThrowsArgumentNullException() + { + var jre = CreateEntry(); + + var ex = Assert.Throws(() => jre.State = null); + Assert.Equal("value", ex.ParamName); + Assert.Equal(JobState.Inactive, jre.State); + } + [Fact] public void SubscribeToStateChange_WhenNull_ThrowsArgumentNullException() { From dc98b7a22ba930e9740c17f90031d0e47ae06079 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 19:59:51 -0700 Subject: [PATCH 04/21] initializer --- .../Services/Jobs/JobRepository.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index 723354fb..78ea6d10 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -317,9 +317,9 @@ public async Task LoadAsync(IReadOnlyList intakeItems, var job = new JobRepositoryEntry { JobModel = envelope.JobModel, - RawJobModel = envelope.RawJobModel + RawJobModel = envelope.RawJobModel, + LastHeartbeatTime = DateTime.UtcNow }; - job.LastHeartbeatTime = DateTime.UtcNow; _inactiveJobsList.Add(job); // Worry about sorting later, see below From 3ddc46b6f45d5d933e07104f24b5e9f62dbaa4ee Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 20:03:18 -0700 Subject: [PATCH 05/21] required --- .../Models/JobRepositoryEntry.cs | 4 ++-- .../Services/Jobs/JobRepository.cs | 3 ++- .../Tests/Models/JobRepositoryEntryTests.cs | 8 ++++++-- .../Services/SourceMessages/SourceMessageSorterTests.cs | 5 ++++- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index e42df01b..419532d5 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -65,7 +65,7 @@ public bool CanHeartbeat } } = true; - public DateTime LastHeartbeatTime + public required DateTime LastHeartbeatTime { get { @@ -83,7 +83,7 @@ public DateTime LastHeartbeatTime } } - public JobState? State + public required JobState? State { get { diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index 78ea6d10..71596dbf 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -318,7 +318,8 @@ public async Task LoadAsync(IReadOnlyList intakeItems, { JobModel = envelope.JobModel, RawJobModel = envelope.RawJobModel, - LastHeartbeatTime = DateTime.UtcNow + LastHeartbeatTime = DateTime.UtcNow, + State = JobState.Inactive }; _inactiveJobsList.Add(job); // Worry about sorting later, see below diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 17188857..89804316 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -11,7 +11,9 @@ private static JobRepositoryEntry CreateEntry() return new JobRepositoryEntry { JobModel = new Mock(MockBehavior.Strict).Object, - RawJobModel = new Mock(MockBehavior.Strict).Object + RawJobModel = new Mock(MockBehavior.Strict).Object, + LastHeartbeatTime = default, + State = JobState.Inactive }; } @@ -103,7 +105,9 @@ public void TestGettersSetters() var jre = new JobRepositoryEntry { JobModel = jobModel, - RawJobModel = rawJobModel + RawJobModel = rawJobModel, + LastHeartbeatTime = default, + State = JobState.Inactive }; Assert.True(jre.CanHeartbeat); diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/SourceMessages/SourceMessageSorterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/SourceMessages/SourceMessageSorterTests.cs index 5440a537..4e926b8b 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/SourceMessages/SourceMessageSorterTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/SourceMessages/SourceMessageSorterTests.cs @@ -1,4 +1,5 @@ using RedShirt.Example.JobWorker.Common.Models; +using RedShirt.Example.JobWorker.Core.Enums; using RedShirt.Example.JobWorker.Core.Models; using RedShirt.Example.JobWorker.Core.Services.SourceMessages; @@ -29,7 +30,9 @@ public void Test_Message_Retention(int numberOfMessages) var output = sorter.GetSortedListOfJobs(items.Select(i => new JobRepositoryEntry { JobModel = i, - RawJobModel = new Mock(MockBehavior.Strict).Object + RawJobModel = new Mock(MockBehavior.Strict).Object, + LastHeartbeatTime = DateTime.UtcNow, + State = JobState.Inactive }).ToList()); Assert.Equal(numberOfMessages, output.Count); From db130601b9d074a3ed74ecc45e63743c2cc7bf59 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 20:05:34 -0700 Subject: [PATCH 06/21] Invoke callback on subscribe --- .../Models/JobRepositoryEntry.cs | 11 +++++++++++ .../Tests/Models/JobRepositoryEntryTests.cs | 7 +++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index 419532d5..0a8eb3b5 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -27,6 +27,10 @@ internal interface IJobRepositoryEntry : ISortableJobWrapper /// Thrown when the setter is given null. JobState? State { get; set; } + /// + /// Register a callback invoked with the current state whenever changes. + /// Invoked immediately with the current state on subscribe when that value is not null. + /// void SubscribeToStateChange(Action action); } @@ -119,9 +123,16 @@ public void SubscribeToStateChange(Action action) { ArgumentNullException.ThrowIfNull(action); + JobState? current; lock (_lock) { _stateChangeCallbacks += action; + current = State; + } + + if (current is { } state) + { + action(state); } } } \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 89804316..687684a2 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -85,14 +85,17 @@ public void SubscribeToStateChange_WhenStateChanges_InvokesCallbacks() var second = new List(); jre.SubscribeToStateChange(first.Add); + Assert.Equal([JobState.Inactive], first); + jre.SubscribeToStateChange(second.Add); + Assert.Equal([JobState.Inactive], second); jre.State = JobState.Active; jre.State = JobState.Active; jre.State = JobState.Complete; - Assert.Equal([JobState.Active, JobState.Complete], first); - Assert.Equal([JobState.Active, JobState.Complete], second); + Assert.Equal([JobState.Inactive, JobState.Active, JobState.Complete], first); + Assert.Equal([JobState.Inactive, JobState.Active, JobState.Complete], second); Assert.Equal(JobState.Complete, jre.State); } From d27f390902295e069f8926c902be66ba9b91eacf Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 20:09:05 -0700 Subject: [PATCH 07/21] Revis SubscribeToState --- .../Models/JobRepositoryEntry.cs | 14 ++++++------ .../Tests/Models/JobRepositoryEntryTests.cs | 22 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index 0a8eb3b5..7e7bdf86 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -28,10 +28,10 @@ internal interface IJobRepositoryEntry : ISortableJobWrapper JobState? State { get; set; } /// - /// Register a callback invoked with the current state whenever changes. - /// Invoked immediately with the current state on subscribe when that value is not null. + /// Register a callback invoked with the current and with later values as they are set. + /// Invoked immediately on subscribe when the current state is not null. /// - void SubscribeToStateChange(Action action); + void SubscribeToState(Action action); } internal sealed class JobRepositoryEntry : IJobRepositoryEntry @@ -41,7 +41,7 @@ internal sealed class JobRepositoryEntry : IJobRepositoryEntry /// private readonly Lock _lock = new(); - private Action? _stateChangeCallbacks; + private Action? _stateCallbacks; public required IRawJobModel RawJobModel { get; init; } public required IJobModel JobModel { get; init; } @@ -112,21 +112,21 @@ public required JobState? State } field = newState; - callbacks = _stateChangeCallbacks; + callbacks = _stateCallbacks; } callbacks?.Invoke(newState); } } = JobState.Inactive; - public void SubscribeToStateChange(Action action) + public void SubscribeToState(Action action) { ArgumentNullException.ThrowIfNull(action); JobState? current; lock (_lock) { - _stateChangeCallbacks += action; + _stateCallbacks += action; current = State; } diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 687684a2..7a839223 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -70,24 +70,16 @@ public void State_WhenSetNull_ThrowsArgumentNullException() } [Fact] - public void SubscribeToStateChange_WhenNull_ThrowsArgumentNullException() - { - var jre = CreateEntry(); - - Assert.Throws(() => jre.SubscribeToStateChange(null!)); - } - - [Fact] - public void SubscribeToStateChange_WhenStateChanges_InvokesCallbacks() + public void SubscribeToState_InvokesWithCurrentValueThenLaterValues() { var jre = CreateEntry(); var first = new List(); var second = new List(); - jre.SubscribeToStateChange(first.Add); + jre.SubscribeToState(first.Add); Assert.Equal([JobState.Inactive], first); - jre.SubscribeToStateChange(second.Add); + jre.SubscribeToState(second.Add); Assert.Equal([JobState.Inactive], second); jre.State = JobState.Active; @@ -99,6 +91,14 @@ public void SubscribeToStateChange_WhenStateChanges_InvokesCallbacks() Assert.Equal(JobState.Complete, jre.State); } + [Fact] + public void SubscribeToState_WhenNull_ThrowsArgumentNullException() + { + var jre = CreateEntry(); + + Assert.Throws(() => jre.SubscribeToState(null!)); + } + [Fact] public void TestGettersSetters() { From f667d78a156b6fb2b634de466b5cdb24a36b3eea Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 20:15:36 -0700 Subject: [PATCH 08/21] Reconsider callback, will need to send old state as well. --- .../Models/JobRepositoryEntry.cs | 22 ++++++++++++------- .../Tests/Models/JobRepositoryEntryTests.cs | 22 +++++++++++-------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index 7e7bdf86..c098a38f 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -28,10 +28,14 @@ internal interface IJobRepositoryEntry : ISortableJobWrapper JobState? State { get; set; } /// - /// Register a callback invoked with the current and with later values as they are set. - /// Invoked immediately on subscribe when the current state is not null. + /// Register a callback invoked with the original and current . + /// Invoked immediately on subscribe when the current state is not null; + /// the original state is null for that first invocation. /// - void SubscribeToState(Action action); + /// + /// Receives the original state (possibly null) and the current non-null state. + /// + void SubscribeToState(Action action); } internal sealed class JobRepositoryEntry : IJobRepositoryEntry @@ -41,7 +45,7 @@ internal sealed class JobRepositoryEntry : IJobRepositoryEntry /// private readonly Lock _lock = new(); - private Action? _stateCallbacks; + private Action? _stateCallbacks; public required IRawJobModel RawJobModel { get; init; } public required IJobModel JobModel { get; init; } @@ -103,7 +107,8 @@ public required JobState? State throw new ArgumentNullException(nameof(value)); } - Action? callbacks; + Action? callbacks; + JobState? original; lock (_lock) { if (field == newState) @@ -111,15 +116,16 @@ public required JobState? State return; } + original = field; field = newState; callbacks = _stateCallbacks; } - callbacks?.Invoke(newState); + callbacks?.Invoke(original, newState); } } = JobState.Inactive; - public void SubscribeToState(Action action) + public void SubscribeToState(Action action) { ArgumentNullException.ThrowIfNull(action); @@ -132,7 +138,7 @@ public void SubscribeToState(Action action) if (current is { } state) { - action(state); + action(null, state); } } } \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 7a839223..73af5227 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -70,24 +70,28 @@ public void State_WhenSetNull_ThrowsArgumentNullException() } [Fact] - public void SubscribeToState_InvokesWithCurrentValueThenLaterValues() + public void SubscribeToState_InvokesWithOriginalAndCurrentValues() { var jre = CreateEntry(); - var first = new List(); - var second = new List(); + var first = new List<(JobState? Original, JobState Current)>(); + var second = new List<(JobState? Original, JobState Current)>(); - jre.SubscribeToState(first.Add); - Assert.Equal([JobState.Inactive], first); + jre.SubscribeToState((original, current) => first.Add((original, current))); + Assert.Equal([(null, JobState.Inactive)], first); - jre.SubscribeToState(second.Add); - Assert.Equal([JobState.Inactive], second); + jre.SubscribeToState((original, current) => second.Add((original, current))); + Assert.Equal([(null, JobState.Inactive)], second); jre.State = JobState.Active; jre.State = JobState.Active; jre.State = JobState.Complete; - Assert.Equal([JobState.Inactive, JobState.Active, JobState.Complete], first); - Assert.Equal([JobState.Inactive, JobState.Active, JobState.Complete], second); + Assert.Equal( + [(null, JobState.Inactive), (JobState.Inactive, JobState.Active), (JobState.Active, JobState.Complete)], + first); + Assert.Equal( + [(null, JobState.Inactive), (JobState.Inactive, JobState.Active), (JobState.Active, JobState.Complete)], + second); Assert.Equal(JobState.Complete, jre.State); } From e12bd8b6e2e1c4cdcb81433c3f31be9ff796d080 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 23:02:11 -0700 Subject: [PATCH 09/21] General progress, mainly splitting end arbiter responsibilities --- .../Extensions/ServiceCollectionExtensions.cs | 11 +- .../Models/JobRepositoryEntry.cs | 28 +- .../AppliedExecutionEndArbiter.cs | 225 ------- .../ExecutorExecutionEndArbiter.cs | 68 ++ .../HeartbeatMonitorExecutionEndArbiter.cs | 162 +++++ .../IdempotencyMonitorExecutionEndArbiter.cs | 186 ++++++ .../Heartbeats/HeartbeatMaintainer.cs | 7 +- .../Idempotency/IdempotencyMonitor.cs | 11 +- .../Services/Jobs/JobExecutor.cs | 2 +- .../Services/Jobs/JobRepository.cs | 209 ++++-- .../Tests/JobResultTranslationTests.cs | 2 +- .../Tests/Models/JobRepositoryEntryTests.cs | 24 +- .../AppliedExecutionEndArbiterTests.cs | 622 ------------------ .../ExecutorExecutionEndArbiterTests.cs | 80 +++ ...eartbeatMonitorExecutionEndArbiterTests.cs | 281 ++++++++ ...mpotencyMonitorExecutionEndArbiterTests.cs | 108 +++ .../Services/Heartbeats/MaintainerTests.cs | 55 +- .../Idempotency/IdempotencyMonitorTests.cs | 52 +- .../Tests/Services/Jobs/JobExecutorTests.cs | 12 +- .../Tests/Services/Jobs/JobRepositoryTests.cs | 131 ++-- 20 files changed, 1211 insertions(+), 1065 deletions(-) delete mode 100644 src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/AppliedExecutionEndArbiter.cs create mode 100644 src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs create mode 100644 src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs create mode 100644 src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs delete mode 100644 test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/AppliedExecutionEndArbiterTests.cs create mode 100644 test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs create mode 100644 test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs create mode 100644 test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs diff --git a/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs b/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs index a73fe76f..7ef91558 100644 --- a/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs +++ b/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs @@ -28,24 +28,23 @@ public static IServiceCollection AddCoreJobManagement(this IServiceCollection se services = services .AddCommon() + // Execution end arbiters + .AddSingleton() + .AddSingleton() + .AddSingleton() + .AddSingleton() // General .AddSingleton() .AddSingleton() .AddSingleton() .AddSingleton() .AddSingleton() - .AddSingleton() - .AddSingleton(provider => - provider.GetRequiredService()) - .AddSingleton(provider => - provider.GetRequiredService()) .AddSingleton() .AddSingleton() .AddSingleton() .AddSingleton() .AddSingleton() .AddSingleton() - .AddSingleton() .AddSingleton() .AddSingleton() .Configure(coreSection) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index c098a38f..e8bb4576 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -26,16 +26,6 @@ internal interface IJobRepositoryEntry : ISortableJobWrapper /// /// Thrown when the setter is given null. JobState? State { get; set; } - - /// - /// Register a callback invoked with the original and current . - /// Invoked immediately on subscribe when the current state is not null; - /// the original state is null for that first invocation. - /// - /// - /// Receives the original state (possibly null) and the current non-null state. - /// - void SubscribeToState(Action action); } internal sealed class JobRepositoryEntry : IJobRepositoryEntry @@ -45,7 +35,7 @@ internal sealed class JobRepositoryEntry : IJobRepositoryEntry /// private readonly Lock _lock = new(); - private Action? _stateCallbacks; + private Action? _stateCallbacks; public required IRawJobModel RawJobModel { get; init; } public required IJobModel JobModel { get; init; } @@ -107,7 +97,7 @@ public required JobState? State throw new ArgumentNullException(nameof(value)); } - Action? callbacks; + Action? callbacks; JobState? original; lock (_lock) { @@ -121,11 +111,19 @@ public required JobState? State callbacks = _stateCallbacks; } - callbacks?.Invoke(original, newState); + callbacks?.Invoke(this, original, newState); } } = JobState.Inactive; - public void SubscribeToState(Action action) + /// + /// Register a callback invoked with this entry plus the original and current . + /// Invoked immediately on subscribe when the current state is not null; + /// the original state is null for that first invocation. + /// + /// + /// Receives this entry, the original state (possibly null), and the current non-null state. + /// + public void SubscribeToState(Action action) { ArgumentNullException.ThrowIfNull(action); @@ -138,7 +136,7 @@ public void SubscribeToState(Action action) if (current is { } state) { - action(null, state); + action(this, null, state); } } } \ No newline at end of file diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/AppliedExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/AppliedExecutionEndArbiter.cs deleted file mode 100644 index de95db5c..00000000 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/AppliedExecutionEndArbiter.cs +++ /dev/null @@ -1,225 +0,0 @@ -using Microsoft.Extensions.Logging; -using RedShirt.Example.JobWorker.Common.Services.Utility; -using RedShirt.Example.JobWorker.Core.Services.Jobs; -using RedShirt.Example.JobWorker.Core.Utility; - -namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; - -/// -/// Dictates if maintainer workers should continue running. -/// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. -/// Initially written as a test-friendly alternative to `while(true){}` -/// -internal interface IAppliedMaintainerExecutionEndArbiter -{ - /// - /// Delays for , honouring both and - /// an internal interrupt signal that triggers when the worker is stopping. - /// If there are no watched jobs to monitor, then also delays until there are watched jobs to monitor before - /// delaying for . - /// Intended for maintainer workers only. - /// Cancellation caused by the internal interrupt signal is ignored and treated as a completed delay. - /// - /// How long to wait when the skip-wait event is not set. - /// Label for future wait-related log messages (unused for now). - /// Description for future wait-related log messages (unused for now). - /// Caller cancellation. - Task MaintainerDelayWaitAsync(TimeSpan delay, string loggerLabel, string loggerDescription, - CancellationToken cancellationToken = default); - - bool MaintainerShouldKeepRunning(); -} - -/// -/// Dictates if executor workers should continue running. -/// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. -/// Written as a test-friendly alternative to `while(true){}` -/// -internal interface IAppliedExecutorExecutionEndArbiter -{ - bool ExecutorsShouldKeepRunning(); -} - -internal sealed class AppliedExecutionEndArbiter : IAppliedMaintainerExecutionEndArbiter, - IAppliedExecutorExecutionEndArbiter, IDisposable -{ - private readonly IExecutionEndArbiter _executionEndArbiter; - private readonly CancellationTokenSource _interruptCts = new(); - private readonly Lock _lock = new(); - private readonly ILogger _logger; - private readonly ISleepService _sleepService; - private readonly AsyncManualResetEvent _watchedJobsToMaintainEvent = new(); - - private bool _disposed; - private int _inactiveJobsCount; - private int _watchedJobsCount; - - /// - /// Centralize decision on whether to send interrupt signal. - /// Unsafe on its own, assumed to be running within a lock statement by the method that invokes it. - /// - private bool ShouldSendMaintainerInterruptSignalUnsafe() - { - return !_executionEndArbiter.ShouldKeepRunning() - && _inactiveJobsCount == 0 - && _watchedJobsCount == 0; - } - - private void TryCancelInterrupt() - { - try - { - _interruptCts.Cancel(); - } - catch (ObjectDisposedException) - { - // Dispose may have already run (e.g. host shutdown); interrupt signalling is best-effort. - } - } - - private void OnInactiveJobChange(int inactiveJobCount) - { - bool shouldInterrupt; - lock (_lock) - { - if (_disposed) - { - return; - } - - _inactiveJobsCount = inactiveJobCount; - shouldInterrupt = ShouldSendMaintainerInterruptSignalUnsafe(); - } - - if (shouldInterrupt) - { - TryCancelInterrupt(); - } - } - - private void OnWatchedJobChange(int watchedJobCount) - { - bool shouldInterrupt; - int previousValue; - - lock (_lock) - { - if (_disposed) - { - return; - } - - previousValue = _watchedJobsCount; - _watchedJobsCount = watchedJobCount; - shouldInterrupt = ShouldSendMaintainerInterruptSignalUnsafe(); - } - - if (previousValue != watchedJobCount) - { - // Confirmed a change - - if (watchedJobCount == 0) - { - _watchedJobsToMaintainEvent.Reset(); - } - else - { - _watchedJobsToMaintainEvent.Set(); - } - } - - if (shouldInterrupt) - { - TryCancelInterrupt(); - } - } - - public AppliedExecutionEndArbiter( - IExecutionEndArbiter executionEndArbiter, - IJobRepository jobRepository, - ISleepService sleepService, - ILogger logger) - { - _executionEndArbiter = executionEndArbiter; - _sleepService = sleepService; - _logger = logger; - jobRepository.SubscribeToInactiveCountUpdate(OnInactiveJobChange); - jobRepository.SubscribeToWatchedJobsUpdate(OnWatchedJobChange); - } - - public bool ExecutorsShouldKeepRunning() - { - // The executor doesn't care about other executors currently processing jobs, so ignoring the watched jobs count. - lock (_lock) - { - return _executionEndArbiter.ShouldKeepRunning() - || _inactiveJobsCount > 0; - } - } - - public async Task MaintainerDelayWaitAsync(TimeSpan delay, string loggerLabel, string loggerDescription, - CancellationToken cancellationToken = default) - { - CancellationToken interruptToken; - lock (_lock) - { - if (_disposed) - { - return; - } - - interruptToken = _interruptCts.Token; - } - - using var linkedCts = - CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, interruptToken); - - try - { - // Immediately check to see if the event is set. - if (!await _watchedJobsToMaintainEvent.WaitAsync(TimeSpan.Zero, linkedCts.Token)) - { - // Event is not set, so wait until it is - // This wait prevents the maintainer from creating noise (trace-level though it may be) - // when there are no watched jobs to maintain. - _logger.LogTrace("{Label}: Waiting for watchable events", loggerLabel); - await _watchedJobsToMaintainEvent.WaitAsync(linkedCts.Token); - return; - } - - _logger.LogTrace("{Label}: {Time} until next {Description}", loggerLabel, delay, - loggerDescription); - await _sleepService.DelayAsync(delay, linkedCts.Token); - } - catch (OperationCanceledException) when (interruptToken.IsCancellationRequested - && !cancellationToken.IsCancellationRequested) - { - // Interrupt-driven cancellation: treat the delay as having elapsed. - } - } - - public bool MaintainerShouldKeepRunning() - { - lock (_lock) - { - return _executionEndArbiter.ShouldKeepRunning() - || _inactiveJobsCount > 0 - || _watchedJobsCount > 0; - } - } - - public void Dispose() - { - lock (_lock) - { - if (_disposed) - { - return; - } - - _disposed = true; - } - - _interruptCts.Dispose(); - } -} \ No newline at end of file diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs new file mode 100644 index 00000000..f69492c3 --- /dev/null +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs @@ -0,0 +1,68 @@ +using Microsoft.Extensions.Logging; +using RedShirt.Example.JobWorker.Common.Services.Utility; +using RedShirt.Example.JobWorker.Core.Services.Jobs; + +namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; + +/// +/// Dictates if executor workers should continue running. +/// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. +/// Written as a test-friendly alternative to `while(true){}` +/// +internal interface IExecutorExecutionEndArbiter +{ + bool ExecutorsShouldKeepRunning(); +} + +internal class ExecutorExecutionEndArbiter : IExecutorExecutionEndArbiter, IDisposable +{ + private readonly IExecutionEndArbiter _executionEndArbiter; + private readonly CancellationTokenSource _interruptCts = new(); + private readonly Lock _lock = new(); + private bool _disposed; + + private int _inactiveJobsCount; + + private void OnInactiveJobCountChange(int inactiveJobCount) + { + lock (_lock) + { + _inactiveJobsCount = inactiveJobCount; + } + } + + private bool ShouldKeepRunningUnsafe() + { + return _executionEndArbiter.ShouldKeepRunning() && _inactiveJobsCount > 0; + } + + public ExecutorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter, + ISleepService sleepService, ILogger logger) + { + _executionEndArbiter = executionEndArbiter; + jobRepository.SubscribeToInactiveCountUpdate(OnInactiveJobCountChange); + } + + public void Dispose() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + } + + _interruptCts.Dispose(); + } + + public bool ExecutorsShouldKeepRunning() + { + lock (_lock) + { + return ShouldKeepRunningUnsafe(); + } + } +} \ No newline at end of file diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs new file mode 100644 index 00000000..28d6cb48 --- /dev/null +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs @@ -0,0 +1,162 @@ +using Microsoft.Extensions.Logging; +using RedShirt.Example.JobWorker.Common.Services.Utility; +using RedShirt.Example.JobWorker.Core.Services.Jobs; +using RedShirt.Example.JobWorker.Core.Utility; + +namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; + +internal interface IHeartbeatMonitorExecutionEndArbiter +{ + /// + /// Delays for , honouring both and + /// an internal interrupt signal that triggers when the worker is stopping. + /// If there are no watched jobs to monitor, then also delays until there are watched jobs to monitor before + /// delaying for . + /// Cancellation caused by the internal interrupt signal is ignored and treated as a completed delay. + /// + /// How long to wait when the skip-wait event is not set. + /// Caller cancellation. + Task HeartbeatMonitorDelayWaitAsync(TimeSpan delay, CancellationToken cancellationToken = default); + + bool MonitorShouldKeepRunning(); +} + +internal class HeartbeatMonitorExecutionEndArbiter : IHeartbeatMonitorExecutionEndArbiter +{ + private const string LogLabel = "Heartbeat Monitor"; + private readonly IExecutionEndArbiter _executionEndArbiter; + private readonly CancellationTokenSource _interruptCts = new(); + private readonly Lock _lock = new(); + private readonly ILogger _logger; + private readonly AsyncManualResetEvent _relevantJobsToObserveEvent = new(); + private readonly ISleepService _sleepService; + private bool _disposed; + private bool _relevantJobsToObserveEventIsActive; + + private int _watchedJobsCount; + + private void OnWatchedJobsCountChange(int watchedJobCount) + { + bool shouldInterrupt; + lock (_lock) + { + if (_disposed) + { + return; + } + + _watchedJobsCount = watchedJobCount; + shouldInterrupt = !ShouldKeepRunningUnsafe(); + ConsiderUpdatingEvent(); + } + + if (shouldInterrupt) + { + TryCancelInterrupt(); + } + } + + private void ConsiderUpdatingEvent() + { + if (_watchedJobsCount == 0) + { + // Set to zero from non-zero + _relevantJobsToObserveEvent.Reset(); + _relevantJobsToObserveEventIsActive = false; + } + else if (!_relevantJobsToObserveEventIsActive) + { + // Set to non-zero, and was not previously active (suggesting from zero) + _relevantJobsToObserveEvent.Set(); + _relevantJobsToObserveEventIsActive = true; + } + } + + private bool ShouldKeepRunningUnsafe() + { + return _executionEndArbiter.ShouldKeepRunning() && _watchedJobsCount > 0; + } + + private void TryCancelInterrupt() + { + try + { + _interruptCts.Cancel(); + } + catch (ObjectDisposedException) + { + // Dispose may have already run (e.g. host shutdown); interrupt signalling is best-effort. + } + } + + public HeartbeatMonitorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter, + ISleepService sleepService, ILogger logger) + { + _executionEndArbiter = executionEndArbiter; + _logger = logger; + _sleepService = sleepService; + jobRepository.SubscribeToWatchedJobsUpdate(OnWatchedJobsCountChange); + } + + public async Task HeartbeatMonitorDelayWaitAsync(TimeSpan delay, CancellationToken cancellationToken = default) + { + CancellationToken interruptToken; + lock (_lock) + { + if (_disposed) + { + return; + } + + interruptToken = _interruptCts.Token; + } + + using var linkedCts = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, interruptToken); + + try + { + // Immediately check to see if the event is set. + if (!await _relevantJobsToObserveEvent.WaitAsync(TimeSpan.Zero, linkedCts.Token)) + { + // Event is not set, so wait until it is + // This wait prevents the maintainer from creating noise (trace-level though it may be) + // when there are no watched jobs to maintain. + _logger.LogTrace("{LogLabel}: Waiting for watchable events", LogLabel); + await _relevantJobsToObserveEvent.WaitAsync(linkedCts.Token); + return; + } + + _logger.LogTrace("{LogLabel}: {Time} until next heartbeat check", LogLabel, delay); + await _sleepService.DelayAsync(delay, linkedCts.Token); + } + catch (OperationCanceledException) when (interruptToken.IsCancellationRequested + && !cancellationToken.IsCancellationRequested) + { + // Interrupt-driven cancellation: treat the delay as having elapsed. + } + } + + public bool MonitorShouldKeepRunning() + { + lock (_lock) + { + return ShouldKeepRunningUnsafe(); + } + } + + public void Dispose() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + } + + _interruptCts.Dispose(); + } +} \ No newline at end of file diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs new file mode 100644 index 00000000..53cd577c --- /dev/null +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs @@ -0,0 +1,186 @@ +using Microsoft.Extensions.Logging; +using RedShirt.Example.JobWorker.Common.Services.Utility; +using RedShirt.Example.JobWorker.Core.Services.Jobs; +using RedShirt.Example.JobWorker.Core.Utility; + +namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; + +internal interface IIdempotencyMonitorExecutionEndArbiter +{ + /// + /// Delays for , honouring both and + /// an internal interrupt signal that triggers when the worker is stopping. + /// If there are no blocked jobs for the idempotency monitor to observe, then also delays until there are + /// before delaying for . + /// Cancellation caused by the internal interrupt signal is ignored and treated as a completed delay. + /// + /// How long to wait when the skip-wait event is not set. + /// Caller cancellation. + Task IdempotencyMonitorDelayWaitAsync(TimeSpan delay, CancellationToken cancellationToken = default); + + bool MonitorShouldKeepRunning(); +} + +internal class IdempotencyMonitorExecutionEndArbiter : IIdempotencyMonitorExecutionEndArbiter +{ + private const string LogLabel = "Idempotency Monitor"; + private readonly IExecutionEndArbiter _executionEndArbiter; + private readonly CancellationTokenSource _interruptCts = new(); + private readonly Lock _lock = new(); + private readonly ILogger _logger; + private readonly AsyncManualResetEvent _relevantJobsToObserveEvent = new(); + private readonly ISleepService _sleepService; + private bool _disposed; + + private int _idempotencyBlockedJobs; + private bool _relevantJobsToObserveEventIsActive; + private int _watchedJobsCount; + + private void OnWatchedJobsCountChange(int watchedJobCount) + { + bool shouldInterrupt; + lock (_lock) + { + if (_disposed) + { + return; + } + + _watchedJobsCount = watchedJobCount; + shouldInterrupt = !ShouldKeepRunningUnsafe(); + ConsiderUpdatingEvent(); + } + + if (shouldInterrupt) + { + TryCancelInterrupt(); + } + } + + private void OnIdempotencyBlockedJobsCountChange(int idempotencyBlockedJobsCount) + { + bool shouldInterrupt; + lock (_lock) + { + if (_disposed) + { + return; + } + + _idempotencyBlockedJobs = idempotencyBlockedJobsCount; + shouldInterrupt = !ShouldKeepRunningUnsafe(); + ConsiderUpdatingEvent(); + } + + if (shouldInterrupt) + { + TryCancelInterrupt(); + } + } + + private void ConsiderUpdatingEvent() + { + if (_watchedJobsCount == 0) + { + // Set to zero from non-zero + _relevantJobsToObserveEvent.Reset(); + _relevantJobsToObserveEventIsActive = false; + } + else if (!_relevantJobsToObserveEventIsActive) + { + // Set to non-zero, and was not previously active (suggesting from zero) + _relevantJobsToObserveEvent.Set(); + _relevantJobsToObserveEventIsActive = true; + } + } + + private bool ShouldKeepRunningUnsafe() + { + return _executionEndArbiter.ShouldKeepRunning() + && _watchedJobsCount > 0; + } + + private void TryCancelInterrupt() + { + try + { + _interruptCts.Cancel(); + } + catch (ObjectDisposedException) + { + // Dispose may have already run (e.g. host shutdown); interrupt signalling is best-effort. + } + } + + public IdempotencyMonitorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter, + ISleepService sleepService, ILogger logger) + { + _executionEndArbiter = executionEndArbiter; + _logger = logger; + _sleepService = sleepService; + jobRepository.SubscribeToWatchedJobsUpdate(OnWatchedJobsCountChange); + jobRepository.SubscribeToIdempotencyBlockedCountUpdate(OnIdempotencyBlockedJobsCountChange); + } + + public bool MonitorShouldKeepRunning() + { + lock (_lock) + { + return ShouldKeepRunningUnsafe(); + } + } + + public async Task IdempotencyMonitorDelayWaitAsync(TimeSpan delay, CancellationToken cancellationToken = default) + { + CancellationToken interruptToken; + lock (_lock) + { + if (_disposed) + { + return; + } + + interruptToken = _interruptCts.Token; + } + + using var linkedCts = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, interruptToken); + + try + { + // Immediately check to see if the event is set. + if (!await _relevantJobsToObserveEvent.WaitAsync(TimeSpan.Zero, linkedCts.Token)) + { + // Event is not set, so wait until it is + // This wait prevents the maintainer from creating noise (trace-level though it may be) + // when there are no watched jobs to maintain. + _logger.LogTrace("{LogLabel}: Waiting for watchable events", LogLabel); + await _relevantJobsToObserveEvent.WaitAsync(linkedCts.Token); + return; + } + + _logger.LogTrace("{LogLabel}: {Time} until next heartbeat check", LogLabel, delay); + await _sleepService.DelayAsync(delay, linkedCts.Token); + } + catch (OperationCanceledException) when (interruptToken.IsCancellationRequested + && !cancellationToken.IsCancellationRequested) + { + // Interrupt-driven cancellation: treat the delay as having elapsed. + } + } + + public void Dispose() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + } + + _interruptCts.Dispose(); + } +} \ No newline at end of file diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs index 80c4f2fb..0ddde6b9 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs @@ -22,7 +22,7 @@ internal interface IHeartbeatMaintainer : IHandlerSubComponent; #pragma warning disable S107 internal sealed class HeartbeatMaintainer( IHeartbeatCalculator heartbeatCalculator, - IAppliedMaintainerExecutionEndArbiter appliedExecutionEndArbiter, + IHeartbeatMonitorExecutionEndArbiter heartbeatExecutionEndArbiter, IJobRepository jobRepository, IJobSource jobSource, ICoreHealthStateUpdateService healthStateUpdateService, @@ -56,8 +56,7 @@ internal sealed class HeartbeatMaintainer( /// private async Task LogAndWaitAsync(TimeSpan timeToWait, CancellationToken cancellationToken = default) { - await appliedExecutionEndArbiter.MaintainerDelayWaitAsync(timeToWait, "Heartbeat Monitor", "heartbeat check", - cancellationToken); + await heartbeatExecutionEndArbiter.HeartbeatMonitorDelayWaitAsync(timeToWait, cancellationToken); } /// @@ -177,7 +176,7 @@ public async Task RunAsync(CancellationToken cancellat return HandlerComponentResponse.NotEnabled; } - while (appliedExecutionEndArbiter.MaintainerShouldKeepRunning()) + while (heartbeatExecutionEndArbiter.MonitorShouldKeepRunning()) { var jobs = await jobRepository.GetAllInFlightJobsAsync(cancellationToken); diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs b/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs index 78d39c8d..d07e3a87 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs @@ -19,7 +19,7 @@ namespace RedShirt.Example.JobWorker.Core.Services.Idempotency; internal interface IIdempotencyMonitor : IHandlerSubComponent; internal sealed class IdempotencyMonitor( - IAppliedMaintainerExecutionEndArbiter executionEndArbiter, + IIdempotencyMonitorExecutionEndArbiter idempotencyMonitorExecutionEndArbiter, IJobRepository jobRepository, IIdempotencyExecutionService idempotencyExecutionService, ISafeJobAcknowledgementService safeJobAcknowledgementService, @@ -97,7 +97,7 @@ await idempotencyExecutionService.SetResultInCacheAsync(blockedJob.RawJobModel, if (unblockedJob is { } jobToUnblock) { /* - * I'm invoking the reload operation in this point in the loop out of fear of an infinite cycle of idempotency monitoring. + * I'm changing the job's state at this point in the loop out of fear of an infinite cycle of idempotency monitoring. * * If the job were reloaded within the idempotency lock, then there would be the potential of a race condition. * If the JobExecutor thread receives job and attempted to acquire an idempotency lock before @@ -107,7 +107,7 @@ await idempotencyExecutionService.SetResultInCacheAsync(blockedJob.RawJobModel, * Instead, we are very deliberately doing this outside of the idempotency lock. */ - await jobRepository.ReloadUnblockedJobAsync(jobToUnblock, cancellationToken); + jobToUnblock.State = JobState.Inactive; } } } @@ -117,8 +117,7 @@ await idempotencyExecutionService.SetResultInCacheAsync(blockedJob.RawJobModel, /// private async Task LogAndWaitAsync(TimeSpan timeToWait, CancellationToken cancellationToken = default) { - await executionEndArbiter.MaintainerDelayWaitAsync(timeToWait, "Idempotency Monitor", "follow-up check", - cancellationToken); + await idempotencyMonitorExecutionEndArbiter.IdempotencyMonitorDelayWaitAsync(timeToWait, cancellationToken); } public async Task RunAsync(CancellationToken cancellationToken = default) @@ -131,7 +130,7 @@ public async Task RunAsync(CancellationToken cancellat var intervalTimeSpan = TimeSpan.FromSeconds(options.Value.EffectiveMonitorIntervalSeconds); - while (executionEndArbiter.MaintainerShouldKeepRunning()) + while (idempotencyMonitorExecutionEndArbiter.MonitorShouldKeepRunning()) { await CheckBlockedJobsAsync(cancellationToken); await LogAndWaitAsync(intervalTimeSpan, cancellationToken); diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs index d93574a2..978ba02a 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobExecutor.cs @@ -25,7 +25,7 @@ internal interface IJobExecutor } internal sealed class JobExecutor( - IAppliedExecutorExecutionEndArbiter appliedExecutionEndArbiter, + IExecutorExecutionEndArbiter appliedExecutionEndArbiter, IJobRepository jobRepository, IIdempotencyExecutionService idempotencyExecutionService, ISafeJobRunner safeJobRunner, diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index 71596dbf..81963dd6 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -36,10 +36,14 @@ internal interface IJobRepository Task LoadAsync(IReadOnlyList intakeItems, CancellationToken cancellationToken = default); - Task ReloadUnblockedJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default); - Task RemoveJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default); + /// + /// Register a callback invoked with the current count of jobs blocked by idempotency whenever + /// that count changes via repository operations. Invoked immediately with the current count on subscribe. + /// + void SubscribeToIdempotencyBlockedCountUpdate(Action callback); + /// /// Register a callback invoked with the current inactive-job count whenever that count changes /// via repository operations. Invoked immediately with the current count on subscribe. @@ -84,6 +88,8 @@ internal sealed class JobRepository( /// private readonly AsyncManualResetEvent _repositoryEmptyEvent = new(true); + private readonly Lock _tallyLock = new(); + /// /// Jobs that have recently been unblocked due to an idempotency lock. /// This queue intended as a shortlist that will jump the normal sorted line of the inactive jobs list. @@ -92,17 +98,24 @@ internal sealed class JobRepository( private readonly SemaphoreSlim _watchedJobsListSemaphore = new(1, 1); + private Action? _idempotencyBlockedJobsCallbacks; + + private int _idempotencyBlockedTally; + private Action? _inactiveCountCallbacks; /// - /// Inactive potential jobs + /// Inactive potential jobs. /// Reminder: This is currently a list instead of a queue because it needs to be sorted in a manner that is consistent - /// with the Batch approach + /// with the Batch approach. /// Similarly, confirming that it is intentional that this list not be marked as readonly. /// private List _inactiveJobsList = []; + private int _inactiveJobsTally; + private Action? _watchedJobsCallbacks; + private int _watchedJobsTally; private void NotifyInactiveCountUpdate(int count) { @@ -115,6 +128,17 @@ private void NotifyInactiveCountUpdate(int count) callbacks?.Invoke(count); } + private void NotifyIdempotencyBlockedCountUpdate(int count) + { + Action? callbacks; + lock (_callbackLock) + { + callbacks = _idempotencyBlockedJobsCallbacks; + } + + callbacks?.Invoke(count); + } + private void NotifyWatchedJobsUpdate(int count) { Action? callbacks; @@ -189,6 +213,121 @@ private async Task TryGetInactiveJobAsync(CancellationToken c }; } + /// + /// Specifically handle transition from idempotency-blocked back to inactive. + /// + /// + /// + /// + private void OnEntryStateUpdateUnblocked(IJobRepositoryEntry job, JobState? oldState, JobState newState) + { + if (oldState != JobState.BlockedByIdempotency || newState != JobState.Inactive) + { + return; + } + + // Identified as a newly-unblocked job. + // Shortlist the job for re-execution in memory. + _unblockedJobsQueue.Enqueue(job); + // Tell any active invocations of GetNextJobAsync that there is something available. + _jobsAvailableEvent.Set(); + } + + /// + /// Handle tally management when an entry changes state. + /// + /// + /// + /// + private void OnEntryStateUpdateTallies(IJobRepositoryEntry job, JobState? oldState, JobState newState) + { + _ = job; + + var updatedWatched = false; + var updatedInactive = false; + var updatedIdempotencyBlocked = false; + + var localTallyWatched = 0; + var localTallyInactive = 0; + var localTallyIdempotencyBlocked = 0; + + lock (_tallyLock) + { + /* Track watched tally */ + + if (oldState is not null && newState == JobState.Complete) + { + // Moving from watched to unwatched + updatedWatched = true; + _watchedJobsTally--; + } + else if (oldState is null && newState != JobState.Complete) + { + // Moving from unwatched to watched + updatedWatched = true; + _watchedJobsTally++; + } + + /* Track individual tallies */ + + // ReSharper disable once SwitchStatementMissingSomeEnumCasesNoDefault + switch (oldState) + { + case JobState.Inactive: + _inactiveJobsTally--; + updatedInactive = true; + break; + case JobState.BlockedByIdempotency: + _idempotencyBlockedTally--; + updatedIdempotencyBlocked = true; + break; + } + + // ReSharper disable once SwitchStatementMissingSomeEnumCasesNoDefault + switch (newState) + { + case JobState.Inactive: + _inactiveJobsTally++; + updatedInactive = true; + break; + case JobState.BlockedByIdempotency: + _idempotencyBlockedTally++; + updatedIdempotencyBlocked = true; + break; + } + + if (updatedInactive) + { + localTallyInactive = _inactiveJobsTally; + } + + if (updatedIdempotencyBlocked) + { + localTallyIdempotencyBlocked = _idempotencyBlockedTally; + } + + if (updatedWatched) + { + localTallyWatched = _watchedJobsTally; + } + } + + if (updatedInactive) + { + NotifyInactiveCountUpdate(localTallyInactive); + } + + if (updatedIdempotencyBlocked) + { + NotifyIdempotencyBlockedCountUpdate(localTallyIdempotencyBlocked); + } + + if (updatedWatched) + { + NotifyWatchedJobsUpdate(localTallyWatched); + } + } + internal List WatchedJobs { get; } = []; public async Task> GetAllInFlightJobsAsync(CancellationToken cancellationToken = default) @@ -289,7 +428,6 @@ public async Task GetWatchedJobsCountAsync(CancellationToken cancellationTo } while (result is null); result.State = JobState.Active; - NotifyInactiveCountUpdate(await GetInactiveJobCountAsync(cancellationToken)); return result; } @@ -321,6 +459,8 @@ public async Task LoadAsync(IReadOnlyList intakeItems, LastHeartbeatTime = DateTime.UtcNow, State = JobState.Inactive }; + job.SubscribeToState(OnEntryStateUpdateTallies); + job.SubscribeToState(OnEntryStateUpdateUnblocked); _inactiveJobsList.Add(job); // Worry about sorting later, see below @@ -353,17 +493,6 @@ public async Task LoadAsync(IReadOnlyList intakeItems, _jobsAvailableEvent.Set(); NotifyWatchedJobsUpdate(await GetWatchedJobsCountAsync(cancellationToken)); - NotifyInactiveCountUpdate(await GetInactiveJobCountAsync(cancellationToken)); - } - - public async Task ReloadUnblockedJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default) - { - job.State = JobState.Inactive; - // Shortlist the job for re-execution in memory - _unblockedJobsQueue.Enqueue(job); - // Tell any active invocations of GetNextJobAsync that there is something available. - _jobsAvailableEvent.Set(); - NotifyInactiveCountUpdate(await GetInactiveJobCountAsync(cancellationToken)); } public async Task RemoveJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default) @@ -402,29 +531,24 @@ public void SubscribeToInactiveCountUpdate(Action callback) _inactiveCountCallbacks += callback; } - /* - * Putting it on the record that I don't particularly like the below implementation on principle. - * It uses a blocking semaphore call, and it duplicates tally logic (especially true for this particular method). - * - * That said, I think the cure would be worse than the disease: - * * Implementing a check specifically for inactive jobs in this method's sibling - * SubscribeToInactiveCountUpdate would need some sort of tracker on the individual - * items changing state that reports in when an item is inactive/non-inactive. - * * Current subscribers do so at instantiation, before the worker - * threads even have a chance to start adding jobs to the repository. - * This suggests that the blocking will be a tiny one-off. - * While this is also a compelling argument for removing the callback - * call at subscription altogether, I think that running it is more intuitive - * (my issues with it aside). - */ - _watchedJobsListSemaphore.Wait(); - try + lock (_tallyLock) { - callback(WatchedJobs.Count(job => job.State == JobState.Inactive)); + callback(_inactiveJobsTally); } - finally + } + + public void SubscribeToIdempotencyBlockedCountUpdate(Action callback) + { + ArgumentNullException.ThrowIfNull(callback); + + lock (_callbackLock) { - _watchedJobsListSemaphore.Release(); + _idempotencyBlockedJobsCallbacks += callback; + } + + lock (_tallyLock) + { + callback(_idempotencyBlockedTally); } } @@ -439,18 +563,7 @@ public void SubscribeToWatchedJobsUpdate(Action callback) /* * Putting it on the record that I don't particularly like the below implementation on principle. - * It uses a blocking semaphore call, and it duplicates tally logic (especially true for sibling method SubscribeToInactiveCountUpdate). - * - * That said, I think the cure would be worse than the disease: - * * Implementing a check specifically for inactive jobs in this method's sibling - * SubscribeToInactiveCountUpdate would need some sort of tracker on the individual - * items changing state that reports in when an item is inactive/non-inactive. - * * Current subscribers do so at instantiation, before the worker - * threads even have a chance to start adding jobs to the repository. - * This suggests that the blocking will be a tiny one-off. - * While this is also a compelling argument for removing the callback - * call at subscription altogether, I think that running it is more intuitive - * (my issues with it aside). + * No matter how brief, I'm always twitchy about using a blocking call to a semaphore wait. Probably for no good reason, though. */ _watchedJobsListSemaphore.Wait(); try diff --git a/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs b/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs index a86696ee..9cb85959 100644 --- a/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.IntegrationTests/Tests/JobResultTranslationTests.cs @@ -95,7 +95,7 @@ public async Task JobResult_FromLogicRunner_IsTranslated_ToJobSource_AndFailureH // Executor loop control: process one job, then stop var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter .Setup(a => a.ExecutorsShouldKeepRunning()) .Returns(() => diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 73af5227..784eb219 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -73,25 +73,33 @@ public void State_WhenSetNull_ThrowsArgumentNullException() public void SubscribeToState_InvokesWithOriginalAndCurrentValues() { var jre = CreateEntry(); - var first = new List<(JobState? Original, JobState Current)>(); - var second = new List<(JobState? Original, JobState Current)>(); + var first = new List<(IJobRepositoryEntry Entry, JobState? Original, JobState Current)>(); + var second = new List<(IJobRepositoryEntry Entry, JobState? Original, JobState Current)>(); - jre.SubscribeToState((original, current) => first.Add((original, current))); - Assert.Equal([(null, JobState.Inactive)], first); + jre.SubscribeToState((entry, original, current) => first.Add((entry, original, current))); + Assert.Equal([(jre, null, JobState.Inactive)], first); - jre.SubscribeToState((original, current) => second.Add((original, current))); - Assert.Equal([(null, JobState.Inactive)], second); + jre.SubscribeToState((entry, original, current) => second.Add((entry, original, current))); + Assert.Equal([(jre, null, JobState.Inactive)], second); jre.State = JobState.Active; jre.State = JobState.Active; jre.State = JobState.Complete; Assert.Equal( - [(null, JobState.Inactive), (JobState.Inactive, JobState.Active), (JobState.Active, JobState.Complete)], + [ + (jre, null, JobState.Inactive), (jre, JobState.Inactive, JobState.Active), + (jre, JobState.Active, JobState.Complete) + ], first); Assert.Equal( - [(null, JobState.Inactive), (JobState.Inactive, JobState.Active), (JobState.Active, JobState.Complete)], + [ + (jre, null, JobState.Inactive), (jre, JobState.Inactive, JobState.Active), + (jre, JobState.Active, JobState.Complete) + ], second); + Assert.All(first, item => Assert.Same(jre, item.Entry)); + Assert.All(second, item => Assert.Same(jre, item.Entry)); Assert.Equal(JobState.Complete, jre.State); } diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/AppliedExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/AppliedExecutionEndArbiterTests.cs deleted file mode 100644 index 23b6ef8d..00000000 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/AppliedExecutionEndArbiterTests.cs +++ /dev/null @@ -1,622 +0,0 @@ -using Microsoft.Extensions.Logging.Abstractions; -using RedShirt.Example.JobWorker.Common.Services.Utility; -using RedShirt.Example.JobWorker.Core.Services.ExecutionState; -using RedShirt.Example.JobWorker.Core.Services.Jobs; -using System.Reflection; - -namespace RedShirt.Example.JobWorker.Core.UnitTests.Tests.Services.ExecutionState; - -public class AppliedExecutionEndArbiterTests -{ - private static Mock CreateSleepService() - { - return new Mock(MockBehavior.Strict); - } - - private static Mock CreateJobRepository(int inactiveCount = 0, int watchedCount = 0) - { - return CreateJobRepository(out _, inactiveCount, watchedCount); - } - - private static Mock CreateJobRepository( - out JobCountNotifier notifier, - int inactiveCount = 0, - int watchedCount = 0) - { - var captured = new JobCountNotifier(); - notifier = captured; - - var jobRepository = new Mock(MockBehavior.Strict); - jobRepository - .Setup(r => r.SubscribeToInactiveCountUpdate(It.IsAny>())) - .Callback>(callback => - { - captured.NotifyInactive = callback; - callback(inactiveCount); - }); - jobRepository - .Setup(r => r.SubscribeToWatchedJobsUpdate(It.IsAny>())) - .Callback>(callback => - { - captured.NotifyWatched = callback; - callback(watchedCount); - }); - return jobRepository; - } - - [Fact] - public void CountCallbacks_AfterDispose_AreIgnored() - { - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - - var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier, 1, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.MaintainerShouldKeepRunning()); - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - - arbiter.Dispose(); - - notifier.NotifyInactive(0); - notifier.NotifyWatched(0); - - // Counts must not change after dispose; keep-running still reflects pre-dispose state. - Assert.True(arbiter.MaintainerShouldKeepRunning()); - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - } - - [Fact] - public void CountCallbacks_UpdateKeepRunningDecisions() - { - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier, 1, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - Assert.True(arbiter.MaintainerShouldKeepRunning()); - - notifier.NotifyInactive(0); - Assert.False(arbiter.ExecutorsShouldKeepRunning()); - Assert.True(arbiter.MaintainerShouldKeepRunning()); - - notifier.NotifyWatched(0); - Assert.False(arbiter.ExecutorsShouldKeepRunning()); - Assert.False(arbiter.MaintainerShouldKeepRunning()); - } - - [Fact] - public void Dispose_IsIdempotent() - { - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - arbiter.Dispose(); - arbiter.Dispose(); - } - - [Fact] - public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoInactive_ReturnsTrue() - { - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(0, 5).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_CompletesNormallyWhenNeitherTokenCancels() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - // Keep jobs present so the interrupt signal is not sent and the wait event is set. - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var sleepService = CreateSleepService(); - sleepService - .Setup(s => s.DelayAsync(delay, It.IsAny())) - .Returns(Task.CompletedTask); - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1, 1).Object, - sleepService.Object, NullLogger.Instance); - - await arbiter.MaintainerDelayWaitAsync(delay, "test", "test", TestContext.Current.CancellationToken); - - sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenCallerCancelsDuringSleep_PropagatesCancellation() - { - var delay = TimeSpan.FromSeconds(5); - using var callerCts = new CancellationTokenSource(); - - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var delayStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var sleepService = CreateSleepService(); - sleepService - .Setup(s => s.DelayAsync(delay, It.IsAny())) - .Returns((TimeSpan _, CancellationToken token) => - { - delayStarted.SetResult(); - return Task.Delay(Timeout.Infinite, token); - }); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(1, 1).Object, - sleepService.Object, NullLogger.Instance); - - var delayTask = arbiter.MaintainerDelayWaitAsync(delay, "test", "test", callerCts.Token); - await delayStarted.Task; - - await callerCts.CancelAsync(); - - await Assert.ThrowsAnyAsync(() => delayTask); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenCallerCancels_PropagatesCancellation() - { - var delay = TimeSpan.FromSeconds(5); - using var callerCts = new CancellationTokenSource(); - await callerCts.CancelAsync(); - - var innerArbiter = new Mock(MockBehavior.Strict); - // Keep jobs present so only the caller token drives cancellation. - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var sleepService = CreateSleepService(); - sleepService - .Setup(s => s.DelayAsync(delay, It.IsAny())) - .Returns((TimeSpan _, CancellationToken token) => Task.FromCanceled(token)); - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1, 1).Object, - sleepService.Object, NullLogger.Instance); - - await Assert.ThrowsAnyAsync(() => - arbiter.MaintainerDelayWaitAsync(delay, "test", "test", callerCts.Token)); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenCountsDropToEmptyWhileStopping_InterruptsAndCompletes() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - // Stopping, but jobs are still present so the interrupt is not sent yet. - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - - var delayStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - CancellationToken linkedToken = default; - - var sleepService = CreateSleepService(); - sleepService - .Setup(s => s.DelayAsync(delay, It.IsAny())) - .Returns((TimeSpan _, CancellationToken token) => - { - linkedToken = token; - delayStarted.SetResult(); - return Task.Delay(Timeout.Infinite, token); - }); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier, 1, 1).Object, - sleepService.Object, NullLogger.Instance); - - var delayTask = arbiter.MaintainerDelayWaitAsync(delay, "test", "test", CancellationToken.None); - await delayStarted.Task; - - // Both counts must be empty before the interrupt fires. - notifier.NotifyInactive(0); - Assert.False(linkedToken.IsCancellationRequested); - - notifier.NotifyWatched(0); - Assert.True(linkedToken.IsCancellationRequested); - - await delayTask; - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenDisposed_ReturnsWithoutSleeping() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var sleepService = CreateSleepService(); - var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1, 1).Object, - sleepService.Object, NullLogger.Instance); - - arbiter.Dispose(); - - await arbiter.MaintainerDelayWaitAsync(delay, "test", "test", CancellationToken.None); - - sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenInterrupted_IgnoresCancellation() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - // Empty job counts while stopping cancels the internal interrupt token on subscribe. - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - - var sleepService = CreateSleepService(); - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository().Object, - sleepService.Object, NullLogger.Instance); - - await arbiter.MaintainerDelayWaitAsync(delay, "test", "test", CancellationToken.None); - - sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenNoWatchedJobsAndKeepRunning_DoesNotInterrupt() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var sleepService = CreateSleepService(); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier).Object, - sleepService.Object, NullLogger.Instance); - - var delayTask = arbiter.MaintainerDelayWaitAsync(delay, "test", "test", CancellationToken.None); - - await Task.Delay(50, TestContext.Current.CancellationToken); - Assert.False(delayTask.IsCompleted); - - // Empty counts while keep-running must not fire the interrupt; only watched jobs unblock. - notifier.NotifyInactive(0); - await Task.Delay(50, TestContext.Current.CancellationToken); - Assert.False(delayTask.IsCompleted); - - notifier.NotifyWatched(1); - await delayTask; - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenNoWatchedJobs_WaitsUntilWatchedThenReturnsWithoutSleeping() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var sleepService = CreateSleepService(); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier).Object, - sleepService.Object, NullLogger.Instance); - - var delayTask = arbiter.MaintainerDelayWaitAsync(delay, "test", "test", CancellationToken.None); - - await Task.Delay(50, TestContext.Current.CancellationToken); - Assert.False(delayTask.IsCompleted); - - notifier.NotifyWatched(1); - await delayTask; - - sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenWaitingForWatched_CallerCancelPropagates() - { - var delay = TimeSpan.FromSeconds(5); - using var callerCts = new CancellationTokenSource(); - - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var sleepService = CreateSleepService(); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository().Object, - sleepService.Object, NullLogger.Instance); - - var delayTask = arbiter.MaintainerDelayWaitAsync(delay, "test", "test", callerCts.Token); - - await Task.Delay(50, TestContext.Current.CancellationToken); - Assert.False(delayTask.IsCompleted); - - await callerCts.CancelAsync(); - - await Assert.ThrowsAnyAsync(() => delayTask); - sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenWaitingForWatched_InterruptCompletesWithoutThrowing() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - // Stopping with inactive work present: interrupt is deferred until counts clear. - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - - var sleepService = CreateSleepService(); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier, 1).Object, - sleepService.Object, NullLogger.Instance); - - var delayTask = arbiter.MaintainerDelayWaitAsync(delay, "test", "test", CancellationToken.None); - - await Task.Delay(50, TestContext.Current.CancellationToken); - Assert.False(delayTask.IsCompleted); - - notifier.NotifyInactive(0); - await delayTask; - - sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); - } - - [Fact(Timeout = 5000)] - public async Task MaintainerDelayWaitAsync_WhenWatchedCountUnchanged_StillTakesSleepPath() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var sleepService = CreateSleepService(); - sleepService - .Setup(s => s.DelayAsync(delay, It.IsAny())) - .Returns(Task.CompletedTask); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier, 0, 1).Object, - sleepService.Object, NullLogger.Instance); - - // Same count must not Reset the wait event; sleep path should remain available. - notifier.NotifyWatched(1); - - await arbiter.MaintainerDelayWaitAsync(delay, "test", "test", CancellationToken.None); - - sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); - } - - [Fact] - public void MaintainerShouldKeepRunning_WhenInnerTrueAndNoJobs_ReturnsTrue() - { - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository().Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.MaintainerShouldKeepRunning()); - } - - /// - /// Test with impossible IJobRepository output - /// - [Fact] - public void TestExecutorStopRunningWeird() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(-1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.False(arbiter.ExecutorsShouldKeepRunning()); - } - - /// - /// All checks return true - /// - [Fact] - public void TestExecutorsKeepRunningA() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(true); - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - } - - [Fact] - public void TestExecutorsKeepRunningBecauseInactive() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - } - - [Fact] - public void TestExecutorsKeepRunningDespiteInner() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - } - - [Fact] - public void TestExecutorsKeepRunningDespiteWatched() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(0, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - // Confirming that we're ignoring watched jobs - Assert.False(arbiter.ExecutorsShouldKeepRunning()); - } - - [Fact] - public void TestExecutorsStopRunning() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository().Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.False(arbiter.ExecutorsShouldKeepRunning()); - } - - /// - /// All checks return true - /// - [Fact] - public void TestMaintainerKeepRunningA() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(true); - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.MaintainerShouldKeepRunning()); - } - - [Fact] - public void TestMaintainerKeepRunningBecauseInactive() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.MaintainerShouldKeepRunning()); - } - - [Fact] - public void TestMaintainerKeepRunningBecauseWatched() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(0, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.MaintainerShouldKeepRunning()); - } - - [Fact] - public void TestMaintainerKeepRunningDespiteInner() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(1, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.True(arbiter.MaintainerShouldKeepRunning()); - } - - [Fact] - public void TestMaintainerStopRunning() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository().Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.False(arbiter.MaintainerShouldKeepRunning()); - } - - /// - /// Test with impossible IJobRepository output - /// - [Fact] - public void TestMaintainerStopRunningWeird() - { - var innerArbiter = new Mock(); - innerArbiter - .Setup(a => a.ShouldKeepRunning()) - .Returns(false); // Inner arbiter says no - - using var arbiter = new AppliedExecutionEndArbiter(innerArbiter.Object, CreateJobRepository(-1, -1).Object, - CreateSleepService().Object, NullLogger.Instance); - - Assert.False(arbiter.MaintainerShouldKeepRunning()); - } - - [Fact] - public void TryCancelInterrupt_WhenCtsAlreadyDisposed_SwallowsObjectDisposedException() - { - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - - using var arbiter = new AppliedExecutionEndArbiter( - innerArbiter.Object, - CreateJobRepository(out var notifier, 1).Object, - CreateSleepService().Object, NullLogger.Instance); - - var field = typeof(AppliedExecutionEndArbiter).GetField("_interruptCts", - BindingFlags.Instance | BindingFlags.NonPublic); - Assert.NotNull(field); - var cts = (CancellationTokenSource) field.GetValue(arbiter)!; - cts.Dispose(); - - // Clearing the last active count would cancel the interrupt CTS; it is already disposed. - notifier.NotifyInactive(0); - - Assert.False(arbiter.MaintainerShouldKeepRunning()); - } - - private sealed class JobCountNotifier - { - public Action NotifyInactive { get; set; } = _ => { }; - public Action NotifyWatched { get; set; } = _ => { }; - } -} \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs new file mode 100644 index 00000000..7ab0263d --- /dev/null +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs @@ -0,0 +1,80 @@ +using Microsoft.Extensions.Logging.Abstractions; +using RedShirt.Example.JobWorker.Common.Services.Utility; +using RedShirt.Example.JobWorker.Core.Services.ExecutionState; +using RedShirt.Example.JobWorker.Core.Services.Jobs; + +namespace RedShirt.Example.JobWorker.Core.UnitTests.Tests.Services.ExecutionState; + +public class ExecutorExecutionEndArbiterTests +{ + private static Mock CreateJobRepository(out Action notifyInactive, int inactiveCount = 0) + { + Action captured = _ => { }; + var jobRepository = new Mock(MockBehavior.Strict); + jobRepository + .Setup(r => r.SubscribeToInactiveCountUpdate(It.IsAny>())) + .Callback>(callback => + { + captured = callback; + callback(inactiveCount); + }); + notifyInactive = count => captured(count); + return jobRepository; + } + + private static ExecutorExecutionEndArbiter CreateArbiter(IExecutionEndArbiter inner, IJobRepository jobRepository) + { + return new ExecutorExecutionEndArbiter(jobRepository, inner, + new Mock(MockBehavior.Strict).Object, + NullLogger.Instance); + } + + [Fact] + public void Dispose_IsIdempotent() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); + arbiter.Dispose(); + } + + [Fact] + public void ExecutorsShouldKeepRunning_WhenInnerFalseAndInactiveJobs_ReturnsFalse() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(false); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); + Assert.False(arbiter.ExecutorsShouldKeepRunning()); + } + + [Fact] + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndInactiveJobs_ReturnsTrue() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); + } + + [Fact] + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoInactive_ReturnsFalse() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 0).Object); + Assert.False(arbiter.ExecutorsShouldKeepRunning()); + } + + [Fact] + public void InactiveCountCallback_UpdatesKeepRunningDecision() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notify, 1).Object); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); + notify(0); + Assert.False(arbiter.ExecutorsShouldKeepRunning()); + notify(2); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); + } +} diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs new file mode 100644 index 00000000..80d19501 --- /dev/null +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs @@ -0,0 +1,281 @@ +using Microsoft.Extensions.Logging.Abstractions; +using RedShirt.Example.JobWorker.Common.Services.Utility; +using RedShirt.Example.JobWorker.Core.Services.ExecutionState; +using RedShirt.Example.JobWorker.Core.Services.Jobs; +using System.Reflection; + +namespace RedShirt.Example.JobWorker.Core.UnitTests.Tests.Services.ExecutionState; + +public class HeartbeatMonitorExecutionEndArbiterTests +{ + private static Mock CreateSleepService() + { + return new Mock(MockBehavior.Strict); + } + + private static Mock CreateJobRepository(out JobCountNotifier notifier, int watchedCount = 0) + { + var captured = new JobCountNotifier(); + notifier = captured; + + var jobRepository = new Mock(MockBehavior.Strict); + jobRepository + .Setup(r => r.SubscribeToWatchedJobsUpdate(It.IsAny>())) + .Callback>(callback => + { + captured.NotifyWatched = callback; + callback(watchedCount); + }); + return jobRepository; + } + + private static Mock CreateJobRepository(int watchedCount = 0) + { + return CreateJobRepository(out _, watchedCount); + } + + private static HeartbeatMonitorExecutionEndArbiter CreateArbiter( + IExecutionEndArbiter inner, + IJobRepository jobRepository, + ISleepService sleepService) + { + return new HeartbeatMonitorExecutionEndArbiter(jobRepository, inner, sleepService, + NullLogger.Instance); + } + + [Fact] + public void CountCallbacks_AfterDispose_AreIgnored() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + CreateSleepService().Object); + + Assert.True(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + notifier.NotifyWatched(0); + Assert.True(arbiter.MonitorShouldKeepRunning()); + } + + [Fact] + public void CountCallbacks_UpdateKeepRunningDecisions() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + CreateSleepService().Object); + + Assert.True(arbiter.MonitorShouldKeepRunning()); + notifier.NotifyWatched(0); + Assert.False(arbiter.MonitorShouldKeepRunning()); + notifier.NotifyWatched(2); + Assert.True(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + } + + [Fact] + public void Dispose_IsIdempotent() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); + arbiter.Dispose(); + arbiter.Dispose(); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_CompletesNormallyWhenNeitherTokenCancels() + { + var delay = TimeSpan.FromSeconds(5); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var sleepService = CreateSleepService(); + sleepService + .Setup(s => s.DelayAsync(delay, It.IsAny())) + .Returns(Task.CompletedTask); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); + await arbiter.HeartbeatMonitorDelayWaitAsync(delay, TestContext.Current.CancellationToken); + sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); + arbiter.Dispose(); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenCallerCancelsDuringSleep_PropagatesCancellation() + { + var delay = TimeSpan.FromSeconds(5); + using var callerCts = new CancellationTokenSource(); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var delayStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var sleepService = CreateSleepService(); + sleepService + .Setup(s => s.DelayAsync(delay, It.IsAny())) + .Returns((TimeSpan _, CancellationToken token) => + { + delayStarted.SetResult(); + return Task.Delay(Timeout.Infinite, token); + }); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); + var delayTask = arbiter.HeartbeatMonitorDelayWaitAsync(delay, callerCts.Token); + await delayStarted.Task; + await callerCts.CancelAsync(); + await Assert.ThrowsAnyAsync(() => delayTask); + arbiter.Dispose(); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenCallerCancels_PropagatesCancellation() + { + var delay = TimeSpan.FromSeconds(5); + using var callerCts = new CancellationTokenSource(); + await callerCts.CancelAsync(); + + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var sleepService = CreateSleepService(); + sleepService + .Setup(s => s.DelayAsync(delay, It.IsAny())) + .Returns((TimeSpan _, CancellationToken token) => Task.FromCanceled(token)); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); + await Assert.ThrowsAnyAsync(() => + arbiter.HeartbeatMonitorDelayWaitAsync(delay, callerCts.Token)); + arbiter.Dispose(); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenWatchedCountDropsToZero_InterruptsAndCompletes() + { + var delay = TimeSpan.FromSeconds(5); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var delayStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + CancellationToken linkedToken = default; + var sleepService = CreateSleepService(); + sleepService + .Setup(s => s.DelayAsync(delay, It.IsAny())) + .Returns((TimeSpan _, CancellationToken token) => + { + linkedToken = token; + delayStarted.SetResult(); + return Task.Delay(Timeout.Infinite, token); + }); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + sleepService.Object); + var delayTask = arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); + await delayStarted.Task; + Assert.False(linkedToken.IsCancellationRequested); + notifier.NotifyWatched(0); + Assert.True(linkedToken.IsCancellationRequested); + await delayTask; + arbiter.Dispose(); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenDisposed_ReturnsWithoutSleeping() + { + var delay = TimeSpan.FromSeconds(5); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + var sleepService = CreateSleepService(); + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); + arbiter.Dispose(); + await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); + sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenInterrupted_IgnoresCancellation() + { + var delay = TimeSpan.FromSeconds(5); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); + var sleepService = CreateSleepService(); + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, sleepService.Object); + await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); + sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); + arbiter.Dispose(); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenWatchedCountUnchanged_StillTakesSleepPath() + { + var delay = TimeSpan.FromSeconds(5); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + var sleepService = CreateSleepService(); + sleepService + .Setup(s => s.DelayAsync(delay, It.IsAny())) + .Returns(Task.CompletedTask); + + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + sleepService.Object); + notifier.NotifyWatched(1); + await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); + sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); + arbiter.Dispose(); + } + + [Fact] + public void MonitorShouldKeepRunning_WhenInnerTrueAndWatchedJobs_ReturnsTrue() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); + Assert.True(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + } + + [Fact] + public void MonitorShouldKeepRunning_WhenInnerTrueAndNoWatchedJobs_ReturnsFalse() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, CreateSleepService().Object); + Assert.False(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + } + + [Fact] + public void MonitorShouldKeepRunning_WhenInnerFalseAndWatchedJobs_ReturnsFalse() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); + Assert.False(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + } + + [Fact] + public void TryCancelInterrupt_WhenCtsAlreadyDisposed_SwallowsObjectDisposedException() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + CreateSleepService().Object); + + var field = typeof(HeartbeatMonitorExecutionEndArbiter).GetField("_interruptCts", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(field); + var cts = (CancellationTokenSource) field.GetValue(arbiter)!; + cts.Dispose(); + notifier.NotifyWatched(0); + Assert.False(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + } + + private sealed class JobCountNotifier + { + public Action NotifyWatched { get; set; } = _ => { }; + } +} diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs new file mode 100644 index 00000000..8e952542 --- /dev/null +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs @@ -0,0 +1,108 @@ +using Microsoft.Extensions.Logging.Abstractions; +using RedShirt.Example.JobWorker.Common.Services.Utility; +using RedShirt.Example.JobWorker.Core.Services.ExecutionState; +using RedShirt.Example.JobWorker.Core.Services.Jobs; + +namespace RedShirt.Example.JobWorker.Core.UnitTests.Tests.Services.ExecutionState; + +public class IdempotencyMonitorExecutionEndArbiterTests +{ + private static Mock CreateJobRepository(out JobCountNotifier notifier, int watchedCount = 0, + int blockedCount = 0) + { + var captured = new JobCountNotifier(); + notifier = captured; + var jobRepository = new Mock(MockBehavior.Strict); + jobRepository + .Setup(r => r.SubscribeToWatchedJobsUpdate(It.IsAny>())) + .Callback>(callback => + { + captured.NotifyWatched = callback; + callback(watchedCount); + }); + jobRepository + .Setup(r => r.SubscribeToIdempotencyBlockedCountUpdate(It.IsAny>())) + .Callback>(callback => + { + captured.NotifyBlocked = callback; + callback(blockedCount); + }); + return jobRepository; + } + + private static IdempotencyMonitorExecutionEndArbiter CreateArbiter(IExecutionEndArbiter inner, + IJobRepository jobRepository, ISleepService? sleepService = null) + { + return new IdempotencyMonitorExecutionEndArbiter(jobRepository, inner, + sleepService ?? new Mock(MockBehavior.Strict).Object, + NullLogger.Instance); + } + + [Fact] + public void CountCallbacks_AfterDispose_AreIgnored() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1, 1).Object); + Assert.True(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + notifier.NotifyWatched(0); + notifier.NotifyBlocked(0); + Assert.True(arbiter.MonitorShouldKeepRunning()); + } + + [Fact] + public void Dispose_IsIdempotent() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); + arbiter.Dispose(); + arbiter.Dispose(); + } + + [Fact(Timeout = 5000)] + public async Task IdempotencyMonitorDelayWaitAsync_CompletesNormallyWhenWatchedJobsExist() + { + var delay = TimeSpan.FromSeconds(5); + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + var sleepService = new Mock(MockBehavior.Strict); + sleepService + .Setup(s => s.DelayAsync(delay, It.IsAny())) + .Returns(Task.CompletedTask); + + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object, sleepService.Object); + await arbiter.IdempotencyMonitorDelayWaitAsync(delay, CancellationToken.None); + sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); + arbiter.Dispose(); + } + + [Fact] + public void MonitorShouldKeepRunning_RequiresInnerTrueAndWatchedJobs() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1).Object); + Assert.True(arbiter.MonitorShouldKeepRunning()); + notifier.NotifyWatched(0); + Assert.False(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + } + + [Fact] + public void MonitorShouldKeepRunning_WhenInnerFalse_ReturnsFalse() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(false); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object); + Assert.False(arbiter.MonitorShouldKeepRunning()); + arbiter.Dispose(); + } + + private sealed class JobCountNotifier + { + public Action NotifyBlocked { get; set; } = _ => { }; + public Action NotifyWatched { get; set; } = _ => { }; + } +} diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs index 79d85dd6..b641e300 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs @@ -32,11 +32,10 @@ private static ISleepService CreateSleepService() return sleepService.Object; } - private static void SetupMaintainerDelay(Mock arbiter) + private static void SetupMaintainerDelay(Mock arbiter) { arbiter - .Setup(a => a.MaintainerDelayWaitAsync(It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny())) + .Setup(a => a.HeartbeatMonitorDelayWaitAsync(It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); } @@ -46,7 +45,7 @@ private static void SetupMaintainerDelay(Mock(MockBehavior.Strict); - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); var jobRepository = new Mock(MockBehavior.Strict); var jobSource = new Mock(MockBehavior.Strict); jobSource.Setup(s => s.RecommendedHeartbeatIntervalSeconds).Returns(intervalSeconds); @@ -84,10 +83,10 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailureFals .Returns(TimeSpan.FromSeconds(1)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -143,10 +142,10 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailure_Pro var heartbeatCalculator = new Mock(); heartbeatCalculator.Setup(c => c.IsReadyForHeartbeat(entry.Object)).Returns(true); - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(true); var jobRepository = new Mock(MockBehavior.Strict); @@ -209,10 +208,10 @@ public async Task TestFilterOutCannotHeartbeatJobs() .Returns(TimeSpan.FromSeconds(1)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -262,10 +261,10 @@ public async Task TestHeartbeatNoJobs() var heartbeatCalculator = new Mock(); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -328,10 +327,10 @@ public async Task TestHeartbeatSingleJob() .Returns(TimeSpan.FromSeconds(1)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -400,10 +399,10 @@ public async Task TestHeartbeatSingleJobButGotHeartbeatException() .Returns(TimeSpan.FromSeconds(1)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -471,10 +470,10 @@ public async Task TestHeartbeatSingleJob_Complete() .Returns(TimeSpan.FromMilliseconds(100)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -530,10 +529,10 @@ public async Task TestHeartbeatSingleJob_ExhaustsTransientRetriesThenDisablesExt heartbeatCalculator.Setup(c => c.TimeUntilNextHeartbeat(entry.Object)).Returns(TimeSpan.FromSeconds(1)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -602,10 +601,10 @@ public async Task TestHeartbeatSingleJob_NotReadyYet() .Returns(TimeSpan.FromMilliseconds(100)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -670,10 +669,10 @@ public async Task TestHeartbeatSingleJob_PreciseTiming() .Returns(TimeSpan.FromMilliseconds(100)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -731,10 +730,10 @@ public async Task TestHeartbeatSingleJob_RetriesTransientFailuresThenSucceeds() heartbeatCalculator.Setup(c => c.TimeUntilNextHeartbeat(entry.Object)).Returns(TimeSpan.FromSeconds(1)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -837,10 +836,10 @@ public async Task TestHeartbeatTwoJob() .Returns(TimeSpan.FromMilliseconds(100)); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs index 70ba3c8f..6259bd1a 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs @@ -55,25 +55,24 @@ private static (Mock Entry, Mock JobModel, Mock< return (entry, jobModel, rawJobModel); } - private static void SetupMaintainerDelay(Mock arbiter) + private static void SetupMaintainerDelay(Mock arbiter) { arbiter - .Setup(a => a.MaintainerDelayWaitAsync(It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny())) + .Setup(a => a.IdempotencyMonitorDelayWaitAsync(It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); } [Fact(Timeout = 1000)] - public async Task RunAsync_PassesWaitLabelAndDescriptionToDelay() + public async Task RunAsync_DelaysUsingEffectiveMonitorInterval() { var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter - .Setup(a => a.MaintainerDelayWaitAsync(TimeSpan.FromSeconds(3), "Idempotency Monitor", "follow-up check", + .Setup(a => a.IdempotencyMonitorDelayWaitAsync(TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken)) .Returns(Task.CompletedTask); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -99,7 +98,7 @@ public async Task RunAsync_PassesWaitLabelAndDescriptionToDelay() await monitor.RunAsync(TestContext.Current.CancellationToken); executionEndArbiter.Verify( - a => a.MaintainerDelayWaitAsync(TimeSpan.FromSeconds(3), "Idempotency Monitor", "follow-up check", + a => a.IdempotencyMonitorDelayWaitAsync(TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken), Times.Once); } @@ -107,13 +106,13 @@ public async Task RunAsync_PassesWaitLabelAndDescriptionToDelay() public async Task RunAsync_SleepsUsingEffectiveMonitorIntervalBetweenLoops() { var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter - .Setup(a => a.MaintainerDelayWaitAsync(TimeSpan.FromSeconds(3), It.IsAny(), It.IsAny(), + .Setup(a => a.IdempotencyMonitorDelayWaitAsync(TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken)) .Returns(Task.CompletedTask); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -139,7 +138,7 @@ public async Task RunAsync_SleepsUsingEffectiveMonitorIntervalBetweenLoops() await monitor.RunAsync(TestContext.Current.CancellationToken); executionEndArbiter.Verify( - a => a.MaintainerDelayWaitAsync(TimeSpan.FromSeconds(3), It.IsAny(), It.IsAny(), + a => a.IdempotencyMonitorDelayWaitAsync(TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken), Times.Once); } @@ -165,10 +164,10 @@ public async Task RunAsync_WhenCachedResultIsNullOrUnsuccessful_ReloadsUnblocked }; var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -184,9 +183,7 @@ public async Task RunAsync_WhenCachedResultIsNullOrUnsuccessful_ReloadsUnblocked jobRepository .Setup(r => r.GetAllIdempotencyBlockedJobsAsync(TestContext.Current.CancellationToken)) .ReturnsAsync([entry.Object]); - jobRepository - .Setup(r => r.ReloadUnblockedJobAsync(entry.Object, TestContext.Current.CancellationToken)) - .Returns(Task.CompletedTask); + entry.SetupSet(e => e.State = JobState.Inactive); var idempotencyExecutionService = new Mock(MockBehavior.Strict); idempotencyExecutionService @@ -203,8 +200,7 @@ public async Task RunAsync_WhenCachedResultIsNullOrUnsuccessful_ReloadsUnblocked await monitor.RunAsync(TestContext.Current.CancellationToken); - jobRepository.Verify(r => r.ReloadUnblockedJobAsync(entry.Object, TestContext.Current.CancellationToken), - Times.Once); + entry.VerifySet(e => e.State = JobState.Inactive, Times.Once); jobRepository.Verify(r => r.RemoveJobAsync(It.IsAny(), It.IsAny()), Times.Never); idempotencyLock.Verify(l => l.UnlockAsync(It.IsAny()), Times.Once); @@ -231,10 +227,10 @@ public async Task RunAsync_WhenCachedResultIsSuccessAndAcknowledgeFails_RemovesJ }; var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -307,10 +303,10 @@ public async Task RunAsync_WhenCachedResultIsSuccessAndAcknowledgeSucceeds_Remov }; var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -367,7 +363,7 @@ public async Task RunAsync_WhenCachedResultIsSuccessAndAcknowledgeSucceeds_Remov public async Task RunAsync_WhenDisabled_ReturnsImmediately() { var monitor = new IdempotencyMonitor( - new Mock(MockBehavior.Strict).Object, + new Mock(MockBehavior.Strict).Object, new Mock(MockBehavior.Strict).Object, new Mock(MockBehavior.Strict).Object, new Mock(MockBehavior.Strict).Object, @@ -383,10 +379,10 @@ public async Task RunAsync_WhenLockNotAcquired_LeavesJobBlocked() var idempotencyLock = CreateLock(false); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); SetupMaintainerDelay(executionEndArbiter); executionEndArbiter - .Setup(a => a.MaintainerShouldKeepRunning()) + .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => { if (doQuit) @@ -416,9 +412,7 @@ public async Task RunAsync_WhenLockNotAcquired_LeavesJobBlocked() await monitor.RunAsync(TestContext.Current.CancellationToken); idempotencyLock.Verify(l => l.UnlockAsync(It.IsAny()), Times.Once); - jobRepository.Verify( - r => r.ReloadUnblockedJobAsync(It.IsAny(), It.IsAny()), - Times.Never); + entry.VerifySet(e => e.State = JobState.Inactive, Times.Never); jobRepository.Verify(r => r.RemoveJobAsync(It.IsAny(), It.IsAny()), Times.Never); } diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs index 9038c6a0..1c3c7e73 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobExecutorTests.cs @@ -70,7 +70,7 @@ public async Task ExecuteSingleJob(CoreJobResult safeRunnerResult) .ReturnsAsync(ackResult); var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter .Setup(a => a.ExecutorsShouldKeepRunning()) .Returns(() => @@ -131,7 +131,7 @@ public async Task ExecuteSingleJob(CoreJobResult safeRunnerResult) public async Task PrepareToExitOnNull() { var doQuit = false; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter .Setup(a => a.ExecutorsShouldKeepRunning()) .Returns(() => @@ -183,7 +183,7 @@ public async Task WhenCachedResultIsSuccessAndAcknowledgeFails_SkipsExecution() }; var calls = 0; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter .Setup(a => a.ExecutorsShouldKeepRunning()) .Returns(() => ++calls <= 1); @@ -253,7 +253,7 @@ public async Task WhenCachedResultIsSuccessAndAcknowledgeSucceeds_SkipsExecution }; var calls = 0; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter .Setup(a => a.ExecutorsShouldKeepRunning()) .Returns(() => ++calls <= 1); @@ -332,7 +332,7 @@ public async Task WhenCachedResultIsUnsuccessful_RunsJobAsRetry(CoreJobResult sa .ReturnsAsync(ackResult); var calls = 0; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter .Setup(a => a.ExecutorsShouldKeepRunning()) .Returns(() => ++calls <= 1); @@ -394,7 +394,7 @@ public async Task WhenIdempotencyLockNotAcquired_MarksJobBlockedAndContinues() idempotencyLock.Setup(l => l.UnlockAsync(It.IsAny())).Returns(Task.CompletedTask); var calls = 0; - var executionEndArbiter = new Mock(MockBehavior.Strict); + var executionEndArbiter = new Mock(MockBehavior.Strict); executionEndArbiter .Setup(a => a.ExecutorsShouldKeepRunning()) .Returns(() => ++calls <= 1); diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs index b6d9686f..723e076e 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs @@ -54,68 +54,6 @@ public async Task LoadAsync_WhenResponseHasNoItems_DoesNotTouchWatchedJobs() Assert.Empty(sorter.Invocations); } - [Fact(Timeout = 2000)] - public async Task ReloadUnblockedJobAsync_ShortlistsJobAheadOfInactiveQueue() - { - var executionEndArbiter = new Mock(MockBehavior.Strict); - executionEndArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - - var jobLoaderStateService = new Mock(MockBehavior.Strict); - var sorter = new Mock(); - sorter - .Setup(s => s.GetSortedListOfJobs(It.IsAny>())) - .Returns((List input) => input); - - var jobRepository = new JobRepository( - executionEndArbiter.Object, - jobLoaderStateService.Object, - sorter.Object, - Options.Create(new JobRepository.ConfigurationModel - { - BacklogSize = 0 - })); - - var queuedModel = new Mock(MockBehavior.Strict); - queuedModel.Setup(m => m.MessageId).Returns("queued"); - var queuedRaw = new Mock(MockBehavior.Strict).Object; - var unblockedModel = new Mock(MockBehavior.Strict); - unblockedModel.Setup(m => m.MessageId).Returns("unblocked"); - - await jobRepository.LoadAsync( - [ - new JobEnvelope - { - JobModel = queuedModel.Object, - RawJobModel = queuedRaw - } - ], - TestContext.Current.CancellationToken); - - var queuedEntry = Assert.Single(jobRepository.WatchedJobs, - j => j.JobModel.MessageId == "queued"); - Assert.Same(queuedRaw, queuedEntry.RawJobModel); - - var blockedEntry = new Mock(MockBehavior.Strict); - blockedEntry.Setup(e => e.JobModel).Returns(unblockedModel.Object); - var blockedState = JobState.BlockedByIdempotency; - blockedEntry.Setup(e => e.State).Returns(() => blockedState); - blockedEntry - .SetupSet(e => e.State = JobState.Inactive) - .Callback(() => blockedState = JobState.Inactive); - blockedEntry - .SetupSet(e => e.State = JobState.Active) - .Callback(() => blockedState = JobState.Active); - jobRepository.WatchedJobs.Add(blockedEntry.Object); - - await jobRepository.ReloadUnblockedJobAsync(blockedEntry.Object, TestContext.Current.CancellationToken); - - var nextJob = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); - - Assert.Same(blockedEntry.Object, nextJob); - blockedEntry.VerifySet(e => e.State = JobState.Inactive, Times.Once); - blockedEntry.VerifySet(e => e.State = JobState.Active, Times.Once); - } - [Fact(Timeout = 2000)] public async Task SubscribeToCountUpdates_InvokesImmediatelyAndOnLoadAndRemove() { @@ -139,13 +77,13 @@ await jobRepository.LoadAsync( ], TestContext.Current.CancellationToken); Assert.Equal([0, 1], inactiveCounts); - Assert.Equal([0, 1], watchedCounts); + Assert.Equal([0, 1, 1], watchedCounts); var job = Assert.Single(jobRepository.WatchedJobs); await jobRepository.RemoveJobAsync(job, TestContext.Current.CancellationToken); Assert.Equal([0, 1, 0], inactiveCounts); - Assert.Equal([0, 1, 0], watchedCounts); + Assert.Equal([0, 1, 1, 0], watchedCounts); } [Fact] @@ -645,7 +583,7 @@ public async Task TestLoadJobsAndWaitForJob_RequeuedBacklog() // Not strictly necessary, but it does imitate the logic of JobExecutor to put the job on the radar of the Idempotency Monitor. gottenJob.State = JobState.BlockedByIdempotency; // Reload the unblocked job, imitating the Idempotency Monitor (this puts it into the queue) - await jobRepository.ReloadUnblockedJobAsync(gottenJob, TestContext.Current.CancellationToken); + gottenJob.State = JobState.Inactive; Assert.Equal(responseSize, await jobRepository.GetWatchedJobsCountAsync(TestContext.Current.CancellationToken)); @@ -769,7 +707,7 @@ public async Task TestLoadJobsAndWaitForJob_RequeuedBacklogThenEmpty() // Not strictly necessary, but it does imitate the logic of JobExecutor to put the job on the radar of the Idempotency Monitor. gottenJob.State = JobState.BlockedByIdempotency; // Reload the unblocked job, imitating the Idempotency Monitor (this puts it into the queue) - await jobRepository.ReloadUnblockedJobAsync(gottenJob, TestContext.Current.CancellationToken); + gottenJob.State = JobState.Inactive; // Wait for another job to finish await manualResetEvent.WaitAsync(TestContext.Current.CancellationToken); @@ -1227,6 +1165,67 @@ public async Task TestWaitForDemand_True() Assert.Empty(sorter.Invocations); } + [Fact(Timeout = 2000)] + public async Task UnblockingJob_ShortlistsJobAheadOfInactiveQueue() + { + var executionEndArbiter = new Mock(MockBehavior.Strict); + executionEndArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + + var jobLoaderStateService = new Mock(MockBehavior.Strict); + var sorter = new Mock(); + sorter + .Setup(s => s.GetSortedListOfJobs(It.IsAny>())) + .Returns((List input) => input); + + var jobRepository = new JobRepository( + executionEndArbiter.Object, + jobLoaderStateService.Object, + sorter.Object, + Options.Create(new JobRepository.ConfigurationModel + { + BacklogSize = 0 + })); + + var unblockedModel = new Mock(MockBehavior.Strict); + unblockedModel.Setup(m => m.MessageId).Returns("unblocked"); + var unblockedRaw = new Mock(MockBehavior.Strict).Object; + var queuedModel = new Mock(MockBehavior.Strict); + queuedModel.Setup(m => m.MessageId).Returns("queued"); + var queuedRaw = new Mock(MockBehavior.Strict).Object; + + await jobRepository.LoadAsync( + [ + new JobEnvelope + { + JobModel = unblockedModel.Object, + RawJobModel = unblockedRaw + } + ], + TestContext.Current.CancellationToken); + + var unblockedEntry = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); + Assert.NotNull(unblockedEntry); + Assert.Equal("unblocked", unblockedEntry.JobModel.MessageId); + + await jobRepository.LoadAsync( + [ + new JobEnvelope + { + JobModel = queuedModel.Object, + RawJobModel = queuedRaw + } + ], + TestContext.Current.CancellationToken); + + unblockedEntry.State = JobState.BlockedByIdempotency; + unblockedEntry.State = JobState.Inactive; + + var nextJob = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); + + Assert.Same(unblockedEntry, nextJob); + Assert.Equal(JobState.Active, nextJob.State); + } + [Fact(Timeout = 2000)] public async Task WaitForEmptyRepositoryAsync_CompletesWhenLastJobRemoved() { From 5e6e8eac31f5f9a1edd4b9a15779132186279f95 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 23:42:26 -0700 Subject: [PATCH 10/21] fixes, documentation --- .../ExecutorExecutionEndArbiter.cs | 51 +++++---- .../HeartbeatMonitorExecutionEndArbiter.cs | 63 +++++++---- .../IdempotencyMonitorExecutionEndArbiter.cs | 52 ++++++--- .../ExecutorExecutionEndArbiterTests.cs | 64 ++++++----- ...eartbeatMonitorExecutionEndArbiterTests.cs | 102 ++++++++---------- ...mpotencyMonitorExecutionEndArbiterTests.cs | 27 +++-- 6 files changed, 215 insertions(+), 144 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs index f69492c3..8f203a83 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs @@ -1,5 +1,3 @@ -using Microsoft.Extensions.Logging; -using RedShirt.Example.JobWorker.Common.Services.Utility; using RedShirt.Example.JobWorker.Core.Services.Jobs; namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; @@ -11,15 +9,18 @@ namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; /// internal interface IExecutorExecutionEndArbiter { + /// + /// Determine whether executor workers should keep running. + /// + /// true if executors should keep running, otherwise false bool ExecutorsShouldKeepRunning(); } -internal class ExecutorExecutionEndArbiter : IExecutorExecutionEndArbiter, IDisposable +internal sealed class ExecutorExecutionEndArbiter : IExecutorExecutionEndArbiter { private readonly IExecutionEndArbiter _executionEndArbiter; - private readonly CancellationTokenSource _interruptCts = new(); private readonly Lock _lock = new(); - private bool _disposed; + private int _idempotencyBlockedJobsCount; private int _inactiveJobsCount; @@ -31,31 +32,35 @@ private void OnInactiveJobCountChange(int inactiveJobCount) } } - private bool ShouldKeepRunningUnsafe() + private void OnIdempotencyBlockedJobsCountChange(int idempotencyBlockedJobsCount) { - return _executionEndArbiter.ShouldKeepRunning() && _inactiveJobsCount > 0; + lock (_lock) + { + _idempotencyBlockedJobsCount = idempotencyBlockedJobsCount; + } } - public ExecutorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter, - ISleepService sleepService, ILogger logger) + /// + /// Determine whether the monitor should keep running. + /// Assumed to be running in a lock statement. + /// + /// true if the monitor should keep running, otherwise false + private bool ShouldKeepRunningUnsafe() { - _executionEndArbiter = executionEndArbiter; - jobRepository.SubscribeToInactiveCountUpdate(OnInactiveJobCountChange); + return _executionEndArbiter.ShouldKeepRunning() + // Tracking inactive jobs + && _inactiveJobsCount > 0 + // Tracking jobs that may become inactive again + && _idempotencyBlockedJobsCount > 0; } - public void Dispose() + public ExecutorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter) { - lock (_lock) - { - if (_disposed) - { - return; - } - - _disposed = true; - } - - _interruptCts.Dispose(); + _executionEndArbiter = executionEndArbiter; + // Track inactive jobs + jobRepository.SubscribeToInactiveCountUpdate(OnInactiveJobCountChange); + // Track jobs that may become inactive again + jobRepository.SubscribeToIdempotencyBlockedCountUpdate(OnIdempotencyBlockedJobsCountChange); } public bool ExecutorsShouldKeepRunning() diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs index 28d6cb48..d31810d6 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs @@ -5,7 +5,12 @@ namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; -internal interface IHeartbeatMonitorExecutionEndArbiter +/// +/// Dictates if the heartbeat monitor should continue running. +/// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. +/// Written as a test-friendly alternative to while(true){} +/// +internal interface IHeartbeatMonitorExecutionEndArbiter : IDisposable { /// /// Delays for , honouring both and @@ -18,10 +23,14 @@ internal interface IHeartbeatMonitorExecutionEndArbiter /// Caller cancellation. Task HeartbeatMonitorDelayWaitAsync(TimeSpan delay, CancellationToken cancellationToken = default); + /// + /// Determine whether the heartbeat monitor should keep running. + /// + /// true if the monitor should keep running, otherwise false bool MonitorShouldKeepRunning(); } -internal class HeartbeatMonitorExecutionEndArbiter : IHeartbeatMonitorExecutionEndArbiter +internal sealed class HeartbeatMonitorExecutionEndArbiter : IHeartbeatMonitorExecutionEndArbiter { private const string LogLabel = "Heartbeat Monitor"; private readonly IExecutionEndArbiter _executionEndArbiter; @@ -72,9 +81,16 @@ private void ConsiderUpdatingEvent() } } + /// + /// Determine whether the monitor should keep running. + /// Assumed to be running in a lock statement. + /// + /// true if the monitor should keep running, otherwise false private bool ShouldKeepRunningUnsafe() { - return _executionEndArbiter.ShouldKeepRunning() && _watchedJobsCount > 0; + return _executionEndArbiter.ShouldKeepRunning() + // All watched jobs need to be under observation for heartbeats + && _watchedJobsCount > 0; } private void TryCancelInterrupt() @@ -89,15 +105,41 @@ private void TryCancelInterrupt() } } + private void Dispose(bool disposing) + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + } + + if (disposing) + { + _interruptCts.Dispose(); + } + } + public HeartbeatMonitorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter, ISleepService sleepService, ILogger logger) { _executionEndArbiter = executionEndArbiter; _logger = logger; _sleepService = sleepService; + // All watched jobs are candidates for heartbeats jobRepository.SubscribeToWatchedJobsUpdate(OnWatchedJobsCountChange); } + public void Dispose() + { + Dispose(true); + // ReSharper disable once GCSuppressFinalizeForTypeWithoutDestructor + GC.SuppressFinalize(this); + } + public async Task HeartbeatMonitorDelayWaitAsync(TimeSpan delay, CancellationToken cancellationToken = default) { CancellationToken interruptToken; @@ -144,19 +186,4 @@ public bool MonitorShouldKeepRunning() return ShouldKeepRunningUnsafe(); } } - - public void Dispose() - { - lock (_lock) - { - if (_disposed) - { - return; - } - - _disposed = true; - } - - _interruptCts.Dispose(); - } } \ No newline at end of file diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs index 53cd577c..2624216e 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs @@ -5,7 +5,12 @@ namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; -internal interface IIdempotencyMonitorExecutionEndArbiter +/// +/// Dictates if the idempotency monitor should continue running. +/// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. +/// Written as a test-friendly alternative to while(true){} +/// +internal interface IIdempotencyMonitorExecutionEndArbiter : IDisposable { /// /// Delays for , honouring both and @@ -18,10 +23,14 @@ internal interface IIdempotencyMonitorExecutionEndArbiter /// Caller cancellation. Task IdempotencyMonitorDelayWaitAsync(TimeSpan delay, CancellationToken cancellationToken = default); + /// + /// Determine whether the idempotency monitor should keep running. + /// + /// true if the monitor should keep running, otherwise false bool MonitorShouldKeepRunning(); } -internal class IdempotencyMonitorExecutionEndArbiter : IIdempotencyMonitorExecutionEndArbiter +internal sealed class IdempotencyMonitorExecutionEndArbiter : IIdempotencyMonitorExecutionEndArbiter { private const string LogLabel = "Idempotency Monitor"; private readonly IExecutionEndArbiter _executionEndArbiter; @@ -80,7 +89,7 @@ private void OnIdempotencyBlockedJobsCountChange(int idempotencyBlockedJobsCount private void ConsiderUpdatingEvent() { - if (_watchedJobsCount == 0) + if (_idempotencyBlockedJobs == 0) { // Set to zero from non-zero _relevantJobsToObserveEvent.Reset(); @@ -94,6 +103,11 @@ private void ConsiderUpdatingEvent() } } + /// + /// Determine whether the monitor should keep running. + /// Assumed to be running in a lock statement. + /// + /// true if the monitor should keep running, otherwise false private bool ShouldKeepRunningUnsafe() { return _executionEndArbiter.ShouldKeepRunning() @@ -112,6 +126,24 @@ private void TryCancelInterrupt() } } + private void Dispose(bool disposing) + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + } + + if (disposing) + { + _interruptCts.Dispose(); + } + } + public IdempotencyMonitorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter, ISleepService sleepService, ILogger logger) { @@ -171,16 +203,8 @@ public async Task IdempotencyMonitorDelayWaitAsync(TimeSpan delay, CancellationT public void Dispose() { - lock (_lock) - { - if (_disposed) - { - return; - } - - _disposed = true; - } - - _interruptCts.Dispose(); + Dispose(true); + // ReSharper disable once GCSuppressFinalizeForTypeWithoutDestructor + GC.SuppressFinalize(this); } } \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs index 7ab0263d..8c593fdb 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs @@ -1,5 +1,3 @@ -using Microsoft.Extensions.Logging.Abstractions; -using RedShirt.Example.JobWorker.Common.Services.Utility; using RedShirt.Example.JobWorker.Core.Services.ExecutionState; using RedShirt.Example.JobWorker.Core.Services.Jobs; @@ -7,74 +5,90 @@ namespace RedShirt.Example.JobWorker.Core.UnitTests.Tests.Services.ExecutionStat public class ExecutorExecutionEndArbiterTests { - private static Mock CreateJobRepository(out Action notifyInactive, int inactiveCount = 0) + private static Mock CreateJobRepository( + out JobCountNotifier notifier, + int inactiveCount = 0, + int blockedCount = 0) { - Action captured = _ => { }; + var captured = new JobCountNotifier(); + notifier = captured; var jobRepository = new Mock(MockBehavior.Strict); jobRepository .Setup(r => r.SubscribeToInactiveCountUpdate(It.IsAny>())) .Callback>(callback => { - captured = callback; + captured.NotifyInactive = callback; callback(inactiveCount); }); - notifyInactive = count => captured(count); + jobRepository + .Setup(r => r.SubscribeToIdempotencyBlockedCountUpdate(It.IsAny>())) + .Callback>(callback => + { + captured.NotifyBlocked = callback; + callback(blockedCount); + }); return jobRepository; } private static ExecutorExecutionEndArbiter CreateArbiter(IExecutionEndArbiter inner, IJobRepository jobRepository) { - return new ExecutorExecutionEndArbiter(jobRepository, inner, - new Mock(MockBehavior.Strict).Object, - NullLogger.Instance); + return new ExecutorExecutionEndArbiter(jobRepository, inner); } [Fact] - public void Dispose_IsIdempotent() + public void CountCallbacks_UpdateKeepRunningDecisions() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); - using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); - arbiter.Dispose(); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1, 1).Object); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); + notifier.NotifyInactive(0); + Assert.False(arbiter.ExecutorsShouldKeepRunning()); + notifier.NotifyInactive(2); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); + notifier.NotifyBlocked(0); + Assert.False(arbiter.ExecutorsShouldKeepRunning()); } [Fact] - public void ExecutorsShouldKeepRunning_WhenInnerFalseAndInactiveJobs_ReturnsFalse() + public void ExecutorsShouldKeepRunning_WhenInnerFalseAndJobsPresent_ReturnsFalse() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(false); - using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object); Assert.False(arbiter.ExecutorsShouldKeepRunning()); } [Fact] - public void ExecutorsShouldKeepRunning_WhenInnerTrueAndInactiveJobs_ReturnsTrue() + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndBothCountsPositive_ReturnsTrue() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); - using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object); Assert.True(arbiter.ExecutorsShouldKeepRunning()); } [Fact] - public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoInactive_ReturnsFalse() + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoBlockedJobs_ReturnsFalse() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); - using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 0).Object); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); Assert.False(arbiter.ExecutorsShouldKeepRunning()); } [Fact] - public void InactiveCountCallback_UpdatesKeepRunningDecision() + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoInactive_ReturnsFalse() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); - using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notify, 1).Object); - Assert.True(arbiter.ExecutorsShouldKeepRunning()); - notify(0); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 0, 1).Object); Assert.False(arbiter.ExecutorsShouldKeepRunning()); - notify(2); - Assert.True(arbiter.ExecutorsShouldKeepRunning()); } -} + + private sealed class JobCountNotifier + { + public Action NotifyBlocked { get; set; } = _ => { }; + public Action NotifyInactive { get; set; } = _ => { }; + } +} \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs index 80d19501..96cbd56d 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs @@ -49,7 +49,7 @@ public void CountCallbacks_AfterDispose_AreIgnored() var innerArbiter = new Mock(MockBehavior.Strict); innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, CreateSleepService().Object); Assert.True(arbiter.MonitorShouldKeepRunning()); @@ -64,7 +64,7 @@ public void CountCallbacks_UpdateKeepRunningDecisions() var innerArbiter = new Mock(MockBehavior.Strict); innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, CreateSleepService().Object); Assert.True(arbiter.MonitorShouldKeepRunning()); @@ -72,7 +72,6 @@ public void CountCallbacks_UpdateKeepRunningDecisions() Assert.False(arbiter.MonitorShouldKeepRunning()); notifier.NotifyWatched(2); Assert.True(arbiter.MonitorShouldKeepRunning()); - arbiter.Dispose(); } [Fact] @@ -81,8 +80,8 @@ public void Dispose_IsIdempotent() var innerArbiter = new Mock(MockBehavior.Strict); innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); - arbiter.Dispose(); + using var arbiter = + CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); arbiter.Dispose(); } @@ -98,10 +97,9 @@ public async Task HeartbeatMonitorDelayWaitAsync_CompletesNormallyWhenNeitherTok .Setup(s => s.DelayAsync(delay, It.IsAny())) .Returns(Task.CompletedTask); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); await arbiter.HeartbeatMonitorDelayWaitAsync(delay, TestContext.Current.CancellationToken); sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); - arbiter.Dispose(); } [Fact(Timeout = 5000)] @@ -122,12 +120,11 @@ public async Task HeartbeatMonitorDelayWaitAsync_WhenCallerCancelsDuringSleep_Pr return Task.Delay(Timeout.Infinite, token); }); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); var delayTask = arbiter.HeartbeatMonitorDelayWaitAsync(delay, callerCts.Token); await delayStarted.Task; await callerCts.CancelAsync(); await Assert.ThrowsAnyAsync(() => delayTask); - arbiter.Dispose(); } [Fact(Timeout = 5000)] @@ -145,10 +142,34 @@ public async Task HeartbeatMonitorDelayWaitAsync_WhenCallerCancels_PropagatesCan .Setup(s => s.DelayAsync(delay, It.IsAny())) .Returns((TimeSpan _, CancellationToken token) => Task.FromCanceled(token)); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); await Assert.ThrowsAnyAsync(() => arbiter.HeartbeatMonitorDelayWaitAsync(delay, callerCts.Token)); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenDisposed_ReturnsWithoutSleeping() + { + var delay = TimeSpan.FromSeconds(5); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + var sleepService = CreateSleepService(); + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); arbiter.Dispose(); + await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); + sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact(Timeout = 5000)] + public async Task HeartbeatMonitorDelayWaitAsync_WhenInterrupted_IgnoresCancellation() + { + var delay = TimeSpan.FromSeconds(5); + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); + var sleepService = CreateSleepService(); + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, sleepService.Object); + await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); + sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); } [Fact(Timeout = 5000)] @@ -170,7 +191,7 @@ public async Task HeartbeatMonitorDelayWaitAsync_WhenWatchedCountDropsToZero_Int return Task.Delay(Timeout.Infinite, token); }); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, sleepService.Object); var delayTask = arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); await delayStarted.Task; @@ -178,33 +199,6 @@ public async Task HeartbeatMonitorDelayWaitAsync_WhenWatchedCountDropsToZero_Int notifier.NotifyWatched(0); Assert.True(linkedToken.IsCancellationRequested); await delayTask; - arbiter.Dispose(); - } - - [Fact(Timeout = 5000)] - public async Task HeartbeatMonitorDelayWaitAsync_WhenDisposed_ReturnsWithoutSleeping() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - var sleepService = CreateSleepService(); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, sleepService.Object); - arbiter.Dispose(); - await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); - sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); - } - - [Fact(Timeout = 5000)] - public async Task HeartbeatMonitorDelayWaitAsync_WhenInterrupted_IgnoresCancellation() - { - var delay = TimeSpan.FromSeconds(5); - var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - var sleepService = CreateSleepService(); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, sleepService.Object); - await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); - sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); - arbiter.Dispose(); } [Fact(Timeout = 5000)] @@ -218,22 +212,21 @@ public async Task HeartbeatMonitorDelayWaitAsync_WhenWatchedCountUnchanged_Still .Setup(s => s.DelayAsync(delay, It.IsAny())) .Returns(Task.CompletedTask); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, sleepService.Object); notifier.NotifyWatched(1); await arbiter.HeartbeatMonitorDelayWaitAsync(delay, CancellationToken.None); sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); - arbiter.Dispose(); } [Fact] - public void MonitorShouldKeepRunning_WhenInnerTrueAndWatchedJobs_ReturnsTrue() + public void MonitorShouldKeepRunning_WhenInnerFalseAndWatchedJobs_ReturnsFalse() { var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); - Assert.True(arbiter.MonitorShouldKeepRunning()); - arbiter.Dispose(); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); + using var arbiter = + CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); + Assert.False(arbiter.MonitorShouldKeepRunning()); } [Fact] @@ -241,19 +234,19 @@ public void MonitorShouldKeepRunning_WhenInnerTrueAndNoWatchedJobs_ReturnsFalse( { var innerArbiter = new Mock(MockBehavior.Strict); innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, CreateSleepService().Object); + using var arbiter = + CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, CreateSleepService().Object); Assert.False(arbiter.MonitorShouldKeepRunning()); - arbiter.Dispose(); } [Fact] - public void MonitorShouldKeepRunning_WhenInnerFalseAndWatchedJobs_ReturnsFalse() + public void MonitorShouldKeepRunning_WhenInnerTrueAndWatchedJobs_ReturnsTrue() { var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); - Assert.False(arbiter.MonitorShouldKeepRunning()); - arbiter.Dispose(); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + using var arbiter = + CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); + Assert.True(arbiter.MonitorShouldKeepRunning()); } [Fact] @@ -261,7 +254,7 @@ public void TryCancelInterrupt_WhenCtsAlreadyDisposed_SwallowsObjectDisposedExce { var innerArbiter = new Mock(MockBehavior.Strict); innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, + using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, CreateSleepService().Object); var field = typeof(HeartbeatMonitorExecutionEndArbiter).GetField("_interruptCts", @@ -271,11 +264,10 @@ public void TryCancelInterrupt_WhenCtsAlreadyDisposed_SwallowsObjectDisposedExce cts.Dispose(); notifier.NotifyWatched(0); Assert.False(arbiter.MonitorShouldKeepRunning()); - arbiter.Dispose(); } private sealed class JobCountNotifier { public Action NotifyWatched { get; set; } = _ => { }; } -} +} \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs index 8e952542..e68cfbf0 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs @@ -43,7 +43,7 @@ public void CountCallbacks_AfterDispose_AreIgnored() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1, 1).Object); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1, 1).Object); Assert.True(arbiter.MonitorShouldKeepRunning()); arbiter.Dispose(); notifier.NotifyWatched(0); @@ -56,8 +56,7 @@ public void Dispose_IsIdempotent() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); - arbiter.Dispose(); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); arbiter.Dispose(); } @@ -72,10 +71,22 @@ public async Task IdempotencyMonitorDelayWaitAsync_CompletesNormallyWhenWatchedJ .Setup(s => s.DelayAsync(delay, It.IsAny())) .Returns(Task.CompletedTask); - var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object, sleepService.Object); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object, sleepService.Object); await arbiter.IdempotencyMonitorDelayWaitAsync(delay, CancellationToken.None); sleepService.Verify(s => s.DelayAsync(delay, It.IsAny()), Times.Once); + } + + [Fact(Timeout = 5000)] + public async Task IdempotencyMonitorDelayWaitAsync_WhenDisposed_ReturnsWithoutSleeping() + { + var delay = TimeSpan.FromSeconds(5); + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + var sleepService = new Mock(MockBehavior.Strict); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object, sleepService.Object); arbiter.Dispose(); + await arbiter.IdempotencyMonitorDelayWaitAsync(delay, CancellationToken.None); + sleepService.Verify(s => s.DelayAsync(It.IsAny(), It.IsAny()), Times.Never); } [Fact] @@ -83,11 +94,10 @@ public void MonitorShouldKeepRunning_RequiresInnerTrueAndWatchedJobs() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); - var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1).Object); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1).Object); Assert.True(arbiter.MonitorShouldKeepRunning()); notifier.NotifyWatched(0); Assert.False(arbiter.MonitorShouldKeepRunning()); - arbiter.Dispose(); } [Fact] @@ -95,9 +105,8 @@ public void MonitorShouldKeepRunning_WhenInnerFalse_ReturnsFalse() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(false); - var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object); Assert.False(arbiter.MonitorShouldKeepRunning()); - arbiter.Dispose(); } private sealed class JobCountNotifier @@ -105,4 +114,4 @@ private sealed class JobCountNotifier public Action NotifyBlocked { get; set; } = _ => { }; public Action NotifyWatched { get; set; } = _ => { }; } -} +} \ No newline at end of file From 8531caf30adb045bf12d57f6197ab3e4a38a6bd6 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 23:44:03 -0700 Subject: [PATCH 11/21] cleanup sweep of Core project and related tests --- .../Services/Jobs/Subscriptions/JobSubscriberIntakeQueue.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/Subscriptions/JobSubscriberIntakeQueue.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/Subscriptions/JobSubscriberIntakeQueue.cs index 456a7651..316d0fa2 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/Subscriptions/JobSubscriberIntakeQueue.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/Subscriptions/JobSubscriberIntakeQueue.cs @@ -57,7 +57,6 @@ public JobSubscriberIntakeQueue(IExecutionEndArbiter executionEndArbiter) { executionEndArbiter.AddOnStopCallback(_ => Cancel()); } - #pragma warning disable S2325 public void Load(IJobSourceResponse jobSourceResponse) #pragma warning disable S2325 From b8e81bb5fdb1ac05e818624c1532ecd96ac07dc3 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 23:48:44 -0700 Subject: [PATCH 12/21] minor documentation sync --- .../Services/ExecutionState/ExecutionEndArbiter.cs | 2 +- .../Services/ExecutionState/ExecutorExecutionEndArbiter.cs | 2 +- .../ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs | 2 +- .../ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutionEndArbiter.cs index 50fc0cd4..6af37e6b 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutionEndArbiter.cs @@ -5,7 +5,7 @@ namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; /// /// Dictates if the app should continue running. -/// Originally written as a test-friendly alternative to `while(true){}` +/// Originally written as a test-friendly alternative to while(true){} /// public interface IExecutionEndArbiter : IDisposable { diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs index 8f203a83..70052f4c 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs @@ -5,7 +5,7 @@ namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; /// /// Dictates if executor workers should continue running. /// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. -/// Written as a test-friendly alternative to `while(true){}` +/// Originally written as a test-friendly alternative to while(true){} /// internal interface IExecutorExecutionEndArbiter { diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs index d31810d6..f690e246 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs @@ -8,7 +8,7 @@ namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; /// /// Dictates if the heartbeat monitor should continue running. /// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. -/// Written as a test-friendly alternative to while(true){} +/// Originally written as a test-friendly alternative to while(true){} /// internal interface IHeartbeatMonitorExecutionEndArbiter : IDisposable { diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs index 2624216e..516038eb 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs @@ -8,7 +8,7 @@ namespace RedShirt.Example.JobWorker.Core.Services.ExecutionState; /// /// Dictates if the idempotency monitor should continue running. /// Extends the functionality of the base IExecutionEndArbiter by accessing the job repository. -/// Written as a test-friendly alternative to while(true){} +/// Originally written as a test-friendly alternative to while(true){} /// internal interface IIdempotencyMonitorExecutionEndArbiter : IDisposable { From 142ccf570702a3c7577ab9860ee12bb3539da640 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Fri, 21 Aug 2026 23:59:12 -0700 Subject: [PATCH 13/21] Mass-rename 'maintainer' references to 'monitor' --- README.md | 2 +- .../Extensions/ServiceCollectionExtensions.cs | 2 +- .../Models/JobRepositoryEntry.cs | 2 +- .../HeartbeatMonitorExecutionEndArbiter.cs | 2 +- .../IdempotencyMonitorExecutionEndArbiter.cs | 2 +- .../Services/Handler.cs | 14 +-- .../Heartbeats/HeartbeatCalculator.cs | 2 +- ...tbeatMaintainer.cs => HeartbeatMonitor.cs} | 12 +- .../Idempotency/IdempotencyMonitor.cs | 2 +- .../Tests/Services/HandlerTests.cs | 68 +++++------ ...ainerTests.cs => HeartbeatMonitorTests.cs} | 106 +++++++++--------- .../Idempotency/IdempotencyMonitorTests.cs | 10 +- 12 files changed, 112 insertions(+), 112 deletions(-) rename src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/{HeartbeatMaintainer.cs => HeartbeatMonitor.cs} (94%) rename test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/{MaintainerTests.cs => HeartbeatMonitorTests.cs} (90%) diff --git a/README.md b/README.md index 78da6bd3..806d6144 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ The general architecture of the core application goes along these lines: * For this general template, the job logic runner implementation is currently a stub that shall sleep for the requested number of seconds. * In the event that the job source implementation requires application-level heartbeats for long-running messages, a - heartbeat maintainer (`IHeartbeatMaintainer`) worker thread shall periodically heartbeat messages according to the job + heartbeat monitor (`IHeartbeatMonitor`) worker thread shall periodically heartbeat messages according to the job source's recommendation. * Generally, job sources that require heartbeats are told to recommend a heartbeat interval of 75% of the maximum in-flight time for a message without heartbeats. For example, an SQS consumer configured with a visibility timeout diff --git a/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs b/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs index 7ef91558..7b3b43ee 100644 --- a/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs +++ b/src/RedShirt.Example.JobWorker.Core/Extensions/ServiceCollectionExtensions.cs @@ -39,7 +39,7 @@ public static IServiceCollection AddCoreJobManagement(this IServiceCollection se .AddSingleton() .AddSingleton() .AddSingleton() - .AddSingleton() + .AddSingleton() .AddSingleton() .AddSingleton() .AddSingleton() diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index e8bb4576..d1392bc1 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -31,7 +31,7 @@ internal interface IJobRepositoryEntry : ISortableJobWrapper internal sealed class JobRepositoryEntry : IJobRepositoryEntry { /// - /// Thread-safety for mutable field access from maintainer, executor, and repository threads. + /// Thread-safety for mutable field access from executor, monitor, and repository threads. /// private readonly Lock _lock = new(); diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs index f690e246..08056cfd 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs @@ -162,7 +162,7 @@ public async Task HeartbeatMonitorDelayWaitAsync(TimeSpan delay, CancellationTok if (!await _relevantJobsToObserveEvent.WaitAsync(TimeSpan.Zero, linkedCts.Token)) { // Event is not set, so wait until it is - // This wait prevents the maintainer from creating noise (trace-level though it may be) + // This wait prevents the monitor from creating noise (trace-level though it may be) // when there are no watched jobs to maintain. _logger.LogTrace("{LogLabel}: Waiting for watchable events", LogLabel); await _relevantJobsToObserveEvent.WaitAsync(linkedCts.Token); diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs index 516038eb..95727cde 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs @@ -184,7 +184,7 @@ public async Task IdempotencyMonitorDelayWaitAsync(TimeSpan delay, CancellationT if (!await _relevantJobsToObserveEvent.WaitAsync(TimeSpan.Zero, linkedCts.Token)) { // Event is not set, so wait until it is - // This wait prevents the maintainer from creating noise (trace-level though it may be) + // This wait prevents the monitor from creating noise (trace-level though it may be) // when there are no watched jobs to maintain. _logger.LogTrace("{LogLabel}: Waiting for watchable events", LogLabel); await _relevantJobsToObserveEvent.WaitAsync(linkedCts.Token); diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Handler.cs b/src/RedShirt.Example.JobWorker.Core/Services/Handler.cs index 6129e030..4d744860 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Handler.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Handler.cs @@ -31,7 +31,7 @@ public interface IHandler /// /// /// -/// +/// /// /// /// @@ -39,7 +39,7 @@ public interface IHandler internal sealed class Handler( IExecutionEndArbiter executionEndArbiter, IJobLoaderLoop jobLoaderLoop, - IHeartbeatMaintainer heartbeatMaintainer, + IHeartbeatMonitor heartbeatMonitor, IJobExecutor jobExecutor, IIdempotencyMonitor idempotencyMonitor, IJobSubscriberManager jobSubscriberManager, @@ -162,13 +162,13 @@ await addToTaskFuncAsync(WorkerThreadType.JobSubscriberManager, } /* - * Note: The Maintainer and Idempotency Monitor tasks are intended to abort immediately if configuration or choice of job source doesn't require them. + * Note: The Heartbeat Monitor and Idempotency Monitor tasks are intended to abort immediately if configuration or choice of job source doesn't require them. * It made for simpler execution in Handler to just run them and add them to the list. */ - // Maintainer thread - await addToTaskFuncAsync(WorkerThreadType.HeartbeatMaintainer, - () => heartbeatMaintainer.RunAsync(cancellationToken)); + // Heartbeat monitor thread + await addToTaskFuncAsync(WorkerThreadType.HeartbeatMonitor, + () => heartbeatMonitor.RunAsync(cancellationToken)); // Idempotency monitor thread await addToTaskFuncAsync(WorkerThreadType.IdempotencyMonitor, @@ -205,7 +205,7 @@ await addToTaskFuncAsync(WorkerThreadType.IdempotencyMonitor, private enum WorkerThreadType { JobExecutor, - HeartbeatMaintainer, + HeartbeatMonitor, IdempotencyMonitor, JobSubscriberManager, MessagePoller diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatCalculator.cs b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatCalculator.cs index 33903f19..f27443a8 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatCalculator.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatCalculator.cs @@ -4,7 +4,7 @@ namespace RedShirt.Example.JobWorker.Core.Services.Heartbeats; /// -/// The abstracted heartbeat checks exist to make reading/testing the code of the Maintainer implementation simpler. +/// The abstracted heartbeat checks exist to make reading/testing the code of the Monitor implementation simpler. /// internal interface IHeartbeatCalculator { diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMonitor.cs similarity index 94% rename from src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs rename to src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMonitor.cs index 0ddde6b9..04dcef1d 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMaintainer.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Heartbeats/HeartbeatMonitor.cs @@ -15,12 +15,12 @@ namespace RedShirt.Example.JobWorker.Core.Services.Heartbeats; /// -/// The maintainer is responsible for making sure that messages checked out from the job source remain 'in flight'. +/// The monitor is responsible for making sure that messages checked out from the job source remain 'in flight'. /// -internal interface IHeartbeatMaintainer : IHandlerSubComponent; +internal interface IHeartbeatMonitor : IHandlerSubComponent; #pragma warning disable S107 -internal sealed class HeartbeatMaintainer( +internal sealed class HeartbeatMonitor( IHeartbeatCalculator heartbeatCalculator, IHeartbeatMonitorExecutionEndArbiter heartbeatExecutionEndArbiter, IJobRepository jobRepository, @@ -28,15 +28,15 @@ internal sealed class HeartbeatMaintainer( ICoreHealthStateUpdateService healthStateUpdateService, ISleepService sleepService, IOptions coreOptions, - ILogger logger) : IHeartbeatMaintainer + ILogger logger) : IHeartbeatMonitor #pragma warning restore S107 { /// /// Set the minimum amount of time to sleep for between loops. - /// The heartbeat maintainer loop uses this to round values up to 500ms to avoid possible inching + /// The heartbeat monitor loop uses this to round values up to 500ms to avoid possible inching /// to the next heartbeat check because of what I think is date comparison imprecision, /// Task.Delay imprecision, or something similar. - /// In local testing, had a situation where the HeartbeatMaintainer slept for 00:00:00.0013576, + /// In local testing, had a situation where the HeartbeatMonitor slept for 00:00:00.0013576, /// then for 00:00:00.0002317, and so on for ~15 more times until it finally reached /// the actual heartbeat threshold. /// Mitigating that issue by setting a minimum. The recommended diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs b/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs index d07e3a87..c459d56e 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Idempotency/IdempotencyMonitor.cs @@ -113,7 +113,7 @@ await idempotencyExecutionService.SetResultInCacheAsync(blockedJob.RawJobModel, } /// - /// Minor centralization of a log message, mirroring HeartbeatMaintainer. + /// Minor centralization of a log message, mirroring HeartbeatMonitor. /// private async Task LogAndWaitAsync(TimeSpan timeToWait, CancellationToken cancellationToken = default) { diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/HandlerTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/HandlerTests.cs index 2fd24d1d..44bce04a 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/HandlerTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/HandlerTests.cs @@ -30,13 +30,13 @@ private static Mock> CreateLogger() private static void SetupNotEnabledWorkers( Mock executor, - Mock maintainer, + Mock monitor, Mock idempotencyMonitor, Mock jobSubscriberManager) { executor.Setup(e => e.RunAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(HandlerComponentResponse.NotEnabled); - maintainer.Setup(m => m.RunAsync(It.IsAny())) + monitor.Setup(m => m.RunAsync(It.IsAny())) .ReturnsAsync(HandlerComponentResponse.NotEnabled); idempotencyMonitor.Setup(m => m.RunAsync(It.IsAny())) .ReturnsAsync(HandlerComponentResponse.NotEnabled); @@ -94,12 +94,12 @@ public async Task HandleAsync_DoesNotCompleteWhenOnlyNotEnabledWorkersFinish() }); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), new NullLogger()); @@ -142,12 +142,12 @@ public async Task HandleAsync_LogsFinishedAndNotEnabledResponses() .ReturnsAsync(HandlerComponentResponse.Finished); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), logger.Object); @@ -157,7 +157,7 @@ public async Task HandleAsync_LogsFinishedAndNotEnabledResponses() Assert.True(result); VerifyWorkerDoneLogged(logger, "MessagePoller", HandlerComponentResponse.Finished, Times.Once()); VerifyWorkerDoneLogged(logger, "JobExecutor", HandlerComponentResponse.NotEnabled, Times.Once()); - VerifyWorkerDoneLogged(logger, "HeartbeatMaintainer", HandlerComponentResponse.NotEnabled, Times.Once()); + VerifyWorkerDoneLogged(logger, "HeartbeatMonitor", HandlerComponentResponse.NotEnabled, Times.Once()); VerifyWorkerDoneLogged(logger, "IdempotencyMonitor", HandlerComponentResponse.NotEnabled, Times.Once()); VerifyWorkerDoneLogged(logger, "JobSubscriberManager", HandlerComponentResponse.NotEnabled, Times.Once()); VerifyWorkerResponseLogged(logger, HandlerComponentResponse.Cancelled, Times.Never()); @@ -179,12 +179,12 @@ public async Task HandleAsync_ReturnsFalseOnUnhandledWorkerException() .ThrowsAsync(expected); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), new NullLogger()); @@ -205,12 +205,12 @@ public async Task HandleAsync_WhenInvokedTwice_ThrowsInvalidOperationException() .ReturnsAsync(HandlerComponentResponse.Finished); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), new NullLogger()); @@ -240,12 +240,12 @@ public async Task HandleAsync_WhenWorkerThrowsOperationCanceledWhileTokenCancele }); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), new NullLogger()); @@ -289,12 +289,12 @@ public async Task HandleAsync_WhenWorkerThrowsOperationCanceledWhileTokenCancele }); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), logger.Object); @@ -331,12 +331,12 @@ public async Task HandleAsync_WhenWorkerThrowsOperationCanceledWhileTokenNotCanc .ThrowsAsync(expected); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), new NullLogger()); @@ -358,12 +358,12 @@ public async Task HandleAsync_WhenWorkerThrowsUnexpectedOperationCanceled_LogsCa .ThrowsAsync(new OperationCanceledException("unexpected cancel")); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), logger.Object); @@ -387,12 +387,12 @@ public async Task HandleAsync_WhenWorkerThrows_LogsExceptionResponse() .ThrowsAsync(new InvalidOperationException("worker blew up")); var executor = new Mock(MockBehavior.Strict); - var maintainer = new Mock(MockBehavior.Strict); + var monitor = new Mock(MockBehavior.Strict); var idempotencyMonitor = new Mock(MockBehavior.Strict); var jobSubscriberManager = new Mock(MockBehavior.Strict); - SetupNotEnabledWorkers(executor, maintainer, idempotencyMonitor, jobSubscriberManager); + SetupNotEnabledWorkers(executor, monitor, idempotencyMonitor, jobSubscriberManager); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = 1}), logger.Object); @@ -420,8 +420,8 @@ public async Task TestRunAsync(int numberOfExecutorThreads, int expectedNumberOf executor.Setup(e => e.RunAsync(It.IsAny(), TestContext.Current.CancellationToken)) .ReturnsAsync(HandlerComponentResponse.Finished); - var maintainer = new Mock(MockBehavior.Strict); - maintainer.Setup(m => m.RunAsync(TestContext.Current.CancellationToken)) + var monitor = new Mock(MockBehavior.Strict); + monitor.Setup(m => m.RunAsync(TestContext.Current.CancellationToken)) .ReturnsAsync(HandlerComponentResponse.NotEnabled); var idempotencyMonitor = new Mock(MockBehavior.Strict); @@ -438,7 +438,7 @@ public async Task TestRunAsync(int numberOfExecutorThreads, int expectedNumberOf }; Assert.Equal(expectedNumberOfThreads, options.EffectiveWorkerThreadCount); - var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, maintainer.Object, executor.Object, + var handler = new Handler(executionEndArbiter, jobLoaderLoop.Object, monitor.Object, executor.Object, idempotencyMonitor.Object, jobSubscriberManager.Object, Options.Create(new ThreadConfigurationModel {WorkerThreadCount = numberOfExecutorThreads}), new NullLogger()); @@ -454,7 +454,7 @@ public async Task TestRunAsync(int numberOfExecutorThreads, int expectedNumberOf executor.Verify(e => e.RunAsync(i1, TestContext.Current.CancellationToken)); } - Assert.Single(maintainer.Invocations); + Assert.Single(monitor.Invocations); Assert.Single(idempotencyMonitor.Invocations); Assert.Single(jobSubscriberManager.Invocations); } diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/HeartbeatMonitorTests.cs similarity index 90% rename from test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs rename to test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/HeartbeatMonitorTests.cs index b641e300..0fb89612 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/MaintainerTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Heartbeats/HeartbeatMonitorTests.cs @@ -14,7 +14,7 @@ namespace RedShirt.Example.JobWorker.Core.UnitTests.Tests.Services.Heartbeats; -public class HeartbeatMaintainerTests +public class HeartbeatMonitorTests { private static ICoreHealthStateUpdateService CreateHealthStateUpdateService() { @@ -32,7 +32,7 @@ private static ISleepService CreateSleepService() return sleepService.Object; } - private static void SetupMaintainerDelay(Mock arbiter) + private static void SetupMonitorDelay(Mock arbiter) { arbiter .Setup(a => a.HeartbeatMonitorDelayWaitAsync(It.IsAny(), It.IsAny())) @@ -50,13 +50,13 @@ public async Task RunAsync_WhenRecommendedHeartbeatIntervalIsNotPositive_Returns var jobSource = new Mock(MockBehavior.Strict); jobSource.Setup(s => s.RecommendedHeartbeatIntervalSeconds).Returns(intervalSeconds); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Empty(executionEndArbiter.Invocations); Assert.Empty(jobRepository.Invocations); @@ -84,7 +84,7 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailureFals var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -112,14 +112,14 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailureFals var health = new Mock(MockBehavior.Strict); health.Setup(h => h.NoteIncident()); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, health.Object, CreateSleepService(), Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), - new NullLogger()); + new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); health.Verify(h => h.NoteIncident(), Times.Once); entry.VerifySet(e => e.CanHeartbeat = false, Times.Once); @@ -143,7 +143,7 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailure_Pro heartbeatCalculator.Setup(c => c.IsReadyForHeartbeat(entry.Object)).Returns(true); var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(true); @@ -162,15 +162,15 @@ public async Task RunAsync_WhenUnexpectedHeartbeatException_AndHaltOnFailure_Pro var health = new Mock(MockBehavior.Strict); health.Setup(h => h.NoteIncident()); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, health.Object, CreateSleepService(), Options.Create(new CoreConfigurationModel {HaltOnFailure = true}), - new NullLogger()); + new NullLogger()); var thrown = await Assert.ThrowsAsync(() => - maintainer.RunAsync(TestContext.Current.CancellationToken)); + monitor.RunAsync(TestContext.Current.CancellationToken)); Assert.Same(unexpected, thrown); health.Verify(h => h.NoteIncident(), Times.Once); @@ -209,7 +209,7 @@ public async Task TestFilterOutCannotHeartbeatJobs() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -239,13 +239,13 @@ public async Task TestFilterOutCannotHeartbeatJobs() .Setup(s => s.HeartbeatAsync(rawJobModel.Object, TestContext.Current.CancellationToken)) .Returns(Task.CompletedTask); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); @@ -262,7 +262,7 @@ public async Task TestHeartbeatNoJobs() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -286,13 +286,13 @@ public async Task TestHeartbeatNoJobs() .Setup(s => s.RecommendedHeartbeatIntervalSeconds) .Returns(1); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); @@ -328,7 +328,7 @@ public async Task TestHeartbeatSingleJob() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -357,13 +357,13 @@ public async Task TestHeartbeatSingleJob() .Setup(s => s.HeartbeatAsync(rawJobModel.Object, TestContext.Current.CancellationToken)) .Returns(Task.CompletedTask); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); @@ -400,7 +400,7 @@ public async Task TestHeartbeatSingleJobButGotHeartbeatException() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -430,13 +430,13 @@ public async Task TestHeartbeatSingleJobButGotHeartbeatException() .Returns(() => throw new WorkerJobSourceException("Test") {CouldBeTransient = false, IsHandled = false, CouldBeExternallySolvable = false}); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); @@ -471,7 +471,7 @@ public async Task TestHeartbeatSingleJob_Complete() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -497,13 +497,13 @@ public async Task TestHeartbeatSingleJob_Complete() .Setup(s => s.RecommendedHeartbeatIntervalSeconds) .Returns(1); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); @@ -530,7 +530,7 @@ public async Task TestHeartbeatSingleJob_ExhaustsTransientRetriesThenDisablesExt var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -561,13 +561,13 @@ public async Task TestHeartbeatSingleJob_ExhaustsTransientRetriesThenDisablesExt .Setup(s => s.DelayAsync(It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), sleepService.Object, - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); jobSource.Verify(s => s.HeartbeatAsync(rawJobModel.Object, TestContext.Current.CancellationToken), Times.Exactly(Globals.HeartbeatRetryCount + 1)); @@ -602,7 +602,7 @@ public async Task TestHeartbeatSingleJob_NotReadyYet() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -628,13 +628,13 @@ public async Task TestHeartbeatSingleJob_NotReadyYet() .Setup(s => s.RecommendedHeartbeatIntervalSeconds) .Returns(1); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); @@ -670,7 +670,7 @@ public async Task TestHeartbeatSingleJob_PreciseTiming() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -699,13 +699,13 @@ public async Task TestHeartbeatSingleJob_PreciseTiming() .Setup(s => s.HeartbeatAsync(rawJobModel.Object, TestContext.Current.CancellationToken)) .Returns(Task.CompletedTask); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); @@ -731,7 +731,7 @@ public async Task TestHeartbeatSingleJob_RetriesTransientFailuresThenSucceeds() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -774,13 +774,13 @@ public async Task TestHeartbeatSingleJob_RetriesTransientFailuresThenSucceeds() .Setup(s => s.DelayAsync(It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), sleepService.Object, - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Equal(3, attempts); entry.VerifySet(e => e.CanHeartbeat = false, Times.Never); @@ -837,7 +837,7 @@ public async Task TestHeartbeatTwoJob() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -870,13 +870,13 @@ public async Task TestHeartbeatTwoJob() .Setup(s => s.HeartbeatAsync(rawJobModel2.Object, TestContext.Current.CancellationToken)) .Returns(Task.CompletedTask); - var maintainer = new HeartbeatMaintainer(heartbeatCalculator.Object, executionEndArbiter.Object, + var monitor = new HeartbeatMonitor(heartbeatCalculator.Object, executionEndArbiter.Object, jobRepository.Object, jobSource.Object, CreateHealthStateUpdateService(), CreateSleepService(), - Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); + Options.Create(new CoreConfigurationModel {HaltOnFailure = false}), new NullLogger()); - await maintainer.RunAsync(TestContext.Current.CancellationToken); + await monitor.RunAsync(TestContext.Current.CancellationToken); Assert.Single(jobRepository.Invocations); diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs index 6259bd1a..3f25c6cf 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Idempotency/IdempotencyMonitorTests.cs @@ -55,7 +55,7 @@ private static (Mock Entry, Mock JobModel, Mock< return (entry, jobModel, rawJobModel); } - private static void SetupMaintainerDelay(Mock arbiter) + private static void SetupMonitorDelay(Mock arbiter) { arbiter .Setup(a => a.IdempotencyMonitorDelayWaitAsync(It.IsAny(), It.IsAny())) @@ -165,7 +165,7 @@ public async Task RunAsync_WhenCachedResultIsNullOrUnsuccessful_ReloadsUnblocked var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -228,7 +228,7 @@ public async Task RunAsync_WhenCachedResultIsSuccessAndAcknowledgeFails_RemovesJ var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -304,7 +304,7 @@ public async Task RunAsync_WhenCachedResultIsSuccessAndAcknowledgeSucceeds_Remov var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => @@ -380,7 +380,7 @@ public async Task RunAsync_WhenLockNotAcquired_LeavesJobBlocked() var doQuit = false; var executionEndArbiter = new Mock(MockBehavior.Strict); - SetupMaintainerDelay(executionEndArbiter); + SetupMonitorDelay(executionEndArbiter); executionEndArbiter .Setup(a => a.MonitorShouldKeepRunning()) .Returns(() => From 8432d43f598208ab5b8492fcd5f54c6461cb43c7 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 00:16:19 -0700 Subject: [PATCH 14/21] logicfix --- .../ExecutorExecutionEndArbiter.cs | 4 +-- .../HeartbeatMonitorExecutionEndArbiter.cs | 2 +- .../IdempotencyMonitorExecutionEndArbiter.cs | 2 +- .../ExecutorExecutionEndArbiterTests.cs | 34 ++++++++++++++----- ...eartbeatMonitorExecutionEndArbiterTests.cs | 24 +++++++++---- ...mpotencyMonitorExecutionEndArbiterTests.cs | 21 ++++++++---- 6 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs index 70052f4c..828b328b 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/ExecutorExecutionEndArbiter.cs @@ -49,9 +49,9 @@ private bool ShouldKeepRunningUnsafe() { return _executionEndArbiter.ShouldKeepRunning() // Tracking inactive jobs - && _inactiveJobsCount > 0 + || _inactiveJobsCount > 0 // Tracking jobs that may become inactive again - && _idempotencyBlockedJobsCount > 0; + || _idempotencyBlockedJobsCount > 0; } public ExecutorExecutionEndArbiter(IJobRepository jobRepository, IExecutionEndArbiter executionEndArbiter) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs index 08056cfd..918ff827 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs @@ -90,7 +90,7 @@ private bool ShouldKeepRunningUnsafe() { return _executionEndArbiter.ShouldKeepRunning() // All watched jobs need to be under observation for heartbeats - && _watchedJobsCount > 0; + || _watchedJobsCount > 0; } private void TryCancelInterrupt() diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs index 95727cde..65612f6c 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs @@ -111,7 +111,7 @@ private void ConsiderUpdatingEvent() private bool ShouldKeepRunningUnsafe() { return _executionEndArbiter.ShouldKeepRunning() - && _watchedJobsCount > 0; + || _watchedJobsCount > 0; } private void TryCancelInterrupt() diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs index 8c593fdb..e8df09cf 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/ExecutorExecutionEndArbiterTests.cs @@ -39,23 +39,32 @@ private static ExecutorExecutionEndArbiter CreateArbiter(IExecutionEndArbiter in public void CountCallbacks_UpdateKeepRunningDecisions() { var inner = new Mock(MockBehavior.Strict); - inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + inner.Setup(a => a.ShouldKeepRunning()).Returns(false); var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1, 1).Object); Assert.True(arbiter.ExecutorsShouldKeepRunning()); notifier.NotifyInactive(0); - Assert.False(arbiter.ExecutorsShouldKeepRunning()); - notifier.NotifyInactive(2); Assert.True(arbiter.ExecutorsShouldKeepRunning()); notifier.NotifyBlocked(0); Assert.False(arbiter.ExecutorsShouldKeepRunning()); + notifier.NotifyInactive(2); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); } [Fact] - public void ExecutorsShouldKeepRunning_WhenInnerFalseAndJobsPresent_ReturnsFalse() + public void ExecutorsShouldKeepRunning_WhenInnerFalseAndJobsPresent_ReturnsTrue() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(false); var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); + } + + [Fact] + public void ExecutorsShouldKeepRunning_WhenInnerFalseAndNoJobs_ReturnsFalse() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(false); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _).Object); Assert.False(arbiter.ExecutorsShouldKeepRunning()); } @@ -69,21 +78,30 @@ public void ExecutorsShouldKeepRunning_WhenInnerTrueAndBothCountsPositive_Return } [Fact] - public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoBlockedJobs_ReturnsFalse() + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoBlockedJobs_ReturnsTrue() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1).Object); - Assert.False(arbiter.ExecutorsShouldKeepRunning()); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); } [Fact] - public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoInactive_ReturnsFalse() + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoInactive_ReturnsTrue() { var inner = new Mock(MockBehavior.Strict); inner.Setup(a => a.ShouldKeepRunning()).Returns(true); var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 0, 1).Object); - Assert.False(arbiter.ExecutorsShouldKeepRunning()); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); + } + + [Fact] + public void ExecutorsShouldKeepRunning_WhenInnerTrueAndNoJobs_ReturnsTrue() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _).Object); + Assert.True(arbiter.ExecutorsShouldKeepRunning()); } private sealed class JobCountNotifier diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs index 96cbd56d..d438ac0b 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiterTests.cs @@ -62,7 +62,7 @@ public void CountCallbacks_AfterDispose_AreIgnored() public void CountCallbacks_UpdateKeepRunningDecisions() { var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, CreateSleepService().Object); @@ -177,7 +177,7 @@ public async Task HeartbeatMonitorDelayWaitAsync_WhenWatchedCountDropsToZero_Int { var delay = TimeSpan.FromSeconds(5); var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); var delayStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); CancellationToken linkedToken = default; @@ -220,23 +220,33 @@ public async Task HeartbeatMonitorDelayWaitAsync_WhenWatchedCountUnchanged_Still } [Fact] - public void MonitorShouldKeepRunning_WhenInnerFalseAndWatchedJobs_ReturnsFalse() + public void MonitorShouldKeepRunning_WhenInnerFalseAndNoWatchedJobs_ReturnsFalse() { var innerArbiter = new Mock(MockBehavior.Strict); innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); using var arbiter = - CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); + CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, CreateSleepService().Object); Assert.False(arbiter.MonitorShouldKeepRunning()); } [Fact] - public void MonitorShouldKeepRunning_WhenInnerTrueAndNoWatchedJobs_ReturnsFalse() + public void MonitorShouldKeepRunning_WhenInnerFalseAndWatchedJobs_ReturnsTrue() + { + var innerArbiter = new Mock(MockBehavior.Strict); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); + using var arbiter = + CreateArbiter(innerArbiter.Object, CreateJobRepository(1).Object, CreateSleepService().Object); + Assert.True(arbiter.MonitorShouldKeepRunning()); + } + + [Fact] + public void MonitorShouldKeepRunning_WhenInnerTrueAndNoWatchedJobs_ReturnsTrue() { var innerArbiter = new Mock(MockBehavior.Strict); innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository().Object, CreateSleepService().Object); - Assert.False(arbiter.MonitorShouldKeepRunning()); + Assert.True(arbiter.MonitorShouldKeepRunning()); } [Fact] @@ -253,7 +263,7 @@ public void MonitorShouldKeepRunning_WhenInnerTrueAndWatchedJobs_ReturnsTrue() public void TryCancelInterrupt_WhenCtsAlreadyDisposed_SwallowsObjectDisposedException() { var innerArbiter = new Mock(MockBehavior.Strict); - innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(true); + innerArbiter.Setup(a => a.ShouldKeepRunning()).Returns(false); using var arbiter = CreateArbiter(innerArbiter.Object, CreateJobRepository(out var notifier, 1).Object, CreateSleepService().Object); diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs index e68cfbf0..756b5b43 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiterTests.cs @@ -90,10 +90,19 @@ public async Task IdempotencyMonitorDelayWaitAsync_WhenDisposed_ReturnsWithoutSl } [Fact] - public void MonitorShouldKeepRunning_RequiresInnerTrueAndWatchedJobs() + public void MonitorShouldKeepRunning_WhenInnerFalseAndNoWatchedJobs_ReturnsFalse() { var inner = new Mock(MockBehavior.Strict); - inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + inner.Setup(a => a.ShouldKeepRunning()).Returns(false); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _).Object); + Assert.False(arbiter.MonitorShouldKeepRunning()); + } + + [Fact] + public void MonitorShouldKeepRunning_WhenInnerFalseAndWatchedJobs_ReturnsTrue() + { + var inner = new Mock(MockBehavior.Strict); + inner.Setup(a => a.ShouldKeepRunning()).Returns(false); using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out var notifier, 1).Object); Assert.True(arbiter.MonitorShouldKeepRunning()); notifier.NotifyWatched(0); @@ -101,12 +110,12 @@ public void MonitorShouldKeepRunning_RequiresInnerTrueAndWatchedJobs() } [Fact] - public void MonitorShouldKeepRunning_WhenInnerFalse_ReturnsFalse() + public void MonitorShouldKeepRunning_WhenInnerTrueAndNoWatchedJobs_ReturnsTrue() { var inner = new Mock(MockBehavior.Strict); - inner.Setup(a => a.ShouldKeepRunning()).Returns(false); - using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _, 1, 1).Object); - Assert.False(arbiter.MonitorShouldKeepRunning()); + inner.Setup(a => a.ShouldKeepRunning()).Returns(true); + using var arbiter = CreateArbiter(inner.Object, CreateJobRepository(out _).Object); + Assert.True(arbiter.MonitorShouldKeepRunning()); } private sealed class JobCountNotifier From bf5c8b5919acba822c77e770a807a793ceb78b9d Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 00:16:56 -0700 Subject: [PATCH 15/21] log-wording --- .../ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs index 65612f6c..a471e884 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs @@ -191,7 +191,7 @@ public async Task IdempotencyMonitorDelayWaitAsync(TimeSpan delay, CancellationT return; } - _logger.LogTrace("{LogLabel}: {Time} until next heartbeat check", LogLabel, delay); + _logger.LogTrace("{LogLabel}: {Time} until next follow-up check", LogLabel, delay); await _sleepService.DelayAsync(delay, linkedCts.Token); } catch (OperationCanceledException) when (interruptToken.IsCancellationRequested From 144be72c33a293c31902455ce4d02311d5fb9cb6 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 00:48:56 -0700 Subject: [PATCH 16/21] rename to unsafe --- .../ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs | 4 ++-- .../ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs index 918ff827..6e912ef2 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/HeartbeatMonitorExecutionEndArbiter.cs @@ -56,7 +56,7 @@ private void OnWatchedJobsCountChange(int watchedJobCount) _watchedJobsCount = watchedJobCount; shouldInterrupt = !ShouldKeepRunningUnsafe(); - ConsiderUpdatingEvent(); + ConsiderUpdatingEventUnsafe(); } if (shouldInterrupt) @@ -65,7 +65,7 @@ private void OnWatchedJobsCountChange(int watchedJobCount) } } - private void ConsiderUpdatingEvent() + private void ConsiderUpdatingEventUnsafe() { if (_watchedJobsCount == 0) { diff --git a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs index a471e884..3de426e4 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/ExecutionState/IdempotencyMonitorExecutionEndArbiter.cs @@ -57,7 +57,7 @@ private void OnWatchedJobsCountChange(int watchedJobCount) _watchedJobsCount = watchedJobCount; shouldInterrupt = !ShouldKeepRunningUnsafe(); - ConsiderUpdatingEvent(); + ConsiderUpdatingEventUnsafe(); } if (shouldInterrupt) @@ -78,7 +78,7 @@ private void OnIdempotencyBlockedJobsCountChange(int idempotencyBlockedJobsCount _idempotencyBlockedJobs = idempotencyBlockedJobsCount; shouldInterrupt = !ShouldKeepRunningUnsafe(); - ConsiderUpdatingEvent(); + ConsiderUpdatingEventUnsafe(); } if (shouldInterrupt) @@ -87,7 +87,7 @@ private void OnIdempotencyBlockedJobsCountChange(int idempotencyBlockedJobsCount } } - private void ConsiderUpdatingEvent() + private void ConsiderUpdatingEventUnsafe() { if (_idempotencyBlockedJobs == 0) { From 5eda61967deca712cad7aa5df67c3ec5c31add04 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 01:19:37 -0700 Subject: [PATCH 17/21] Make sure that IJobRepositoryEntry state is Complete before removing from repo. --- .../Services/Jobs/JobRepository.cs | 20 +++++++++---------- .../Tests/Services/Jobs/JobRepositoryTests.cs | 10 +++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index 81963dd6..c53429a6 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -257,13 +257,13 @@ private void OnEntryStateUpdateTallies(IJobRepositoryEntry job, JobState? oldSta if (oldState is not null && newState == JobState.Complete) { - // Moving from watched to unwatched + // Moving from watched to unwatched (as opposed to directly to Complete, which would skip watching altogether) updatedWatched = true; _watchedJobsTally--; } else if (oldState is null && newState != JobState.Complete) { - // Moving from unwatched to watched + // Moving from unwatched to watched (as opposed to directly to Complete) updatedWatched = true; _watchedJobsTally++; } @@ -497,19 +497,22 @@ public async Task LoadAsync(IReadOnlyList intakeItems, public async Task RemoveJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default) { - int watchedCount; - int inactiveCount; + // Confirm that the job that we're removing is marked as complete, + // for the sake of subscriber callbacks in the underlying JobRepositoryEntry. + job.State = JobState.Complete; + await _watchedJobsListSemaphore.WaitAsync(cancellationToken); try { WatchedJobs.Remove(job); - watchedCount = WatchedJobs.Count; - inactiveCount = WatchedJobs.Count(watchedJob => watchedJob.State == JobState.Inactive); - if (watchedCount == 0) + if (WatchedJobs.Count == 0) { // Avoid possible race condition in JobLoader + + // If there's nothing to grab, then there must be an executor about to demand something. _jobsDemandEvent.Set(); + // No watched jobs is the very definition of an empty repository _repositoryEmptyEvent.Set(); } } @@ -517,9 +520,6 @@ public async Task RemoveJobAsync(IJobRepositoryEntry job, CancellationToken canc { _watchedJobsListSemaphore.Release(); } - - NotifyWatchedJobsUpdate(watchedCount); - NotifyInactiveCountUpdate(inactiveCount); } public void SubscribeToInactiveCountUpdate(Action callback) diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs index 723e076e..f1a1dafb 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs @@ -81,6 +81,7 @@ await jobRepository.LoadAsync( var job = Assert.Single(jobRepository.WatchedJobs); await jobRepository.RemoveJobAsync(job, TestContext.Current.CancellationToken); + Assert.Equal(JobState.Complete, job.State); Assert.Equal([0, 1, 0], inactiveCounts); Assert.Equal([0, 1, 1, 0], watchedCounts); @@ -1036,7 +1037,7 @@ public async Task TestRemoveJobsAsync() Options.Create(options)); var job = new Mock(); - job.Setup(j => j.State).Returns(JobState.Complete); + job.SetupProperty(j => j.State, JobState.Active); jobRepository.WatchedJobs.Add(job.Object); Assert.Equal(0, await jobRepository.GetInactiveJobCountAsync(TestContext.Current.CancellationToken)); @@ -1044,6 +1045,7 @@ public async Task TestRemoveJobsAsync() await jobRepository.RemoveJobAsync(job.Object, TestContext.Current.CancellationToken); + Assert.Equal(JobState.Complete, job.Object.State); Assert.Equal(0, await jobRepository.GetWatchedJobsCountAsync(TestContext.Current.CancellationToken)); } @@ -1071,11 +1073,11 @@ public async Task TestRemoveJobsAsyncB() Options.Create(options)); var job = new Mock(); - job.Setup(j => j.State).Returns(JobState.Complete); + job.SetupProperty(j => j.State, JobState.Active); jobRepository.WatchedJobs.Add(job.Object); var job2 = new Mock(); - job2.Setup(j => j.State).Returns(JobState.Complete); + job2.SetupProperty(j => j.State, JobState.Active); jobRepository.WatchedJobs.Add(job2.Object); Assert.Equal(0, await jobRepository.GetInactiveJobCountAsync(TestContext.Current.CancellationToken)); @@ -1083,6 +1085,8 @@ public async Task TestRemoveJobsAsyncB() await jobRepository.RemoveJobAsync(job.Object, TestContext.Current.CancellationToken); + Assert.Equal(JobState.Complete, job.Object.State); + Assert.Equal(JobState.Active, job2.Object.State); Assert.Equal(1, await jobRepository.GetWatchedJobsCountAsync(TestContext.Current.CancellationToken)); } From 6712d1ab697dbd519d481958142a67dd8fe76993 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 10:36:16 -0700 Subject: [PATCH 18/21] Pivot to Dispose --- .../Models/JobRepositoryEntry.cs | 59 ++++++++++- .../Services/Jobs/JobRepository.cs | 64 +++++++++--- .../Tests/Models/JobRepositoryEntryTests.cs | 20 ++++ .../Tests/Services/Jobs/JobRepositoryTests.cs | 99 +++++++++++++++++++ 4 files changed, 226 insertions(+), 16 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs index d1392bc1..abc34fe6 100644 --- a/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs +++ b/src/RedShirt.Example.JobWorker.Core/Models/JobRepositoryEntry.cs @@ -8,8 +8,9 @@ internal interface ISortableJobWrapper IJobModel JobModel { get; } } -internal interface IJobRepositoryEntry : ISortableJobWrapper +internal interface IJobRepositoryEntry : ISortableJobWrapper, IDisposable { + bool IsDisposed { get; } IRawJobModel RawJobModel { get; } /// @@ -35,8 +36,58 @@ internal sealed class JobRepositoryEntry : IJobRepositoryEntry /// private readonly Lock _lock = new(); + private bool _disposed; + private Action? _stateCallbacks; + private void Dispose(bool disposing) + { + if (!disposing) + { + return; + } + + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + } + + // Confirm that the job that we're removing is marked as complete, + // for the sake of subscriber callbacks in the underlying JobRepositoryEntry. + State = JobState.Complete; + + lock (_lock) + { + _stateCallbacks = null; + } + } + + /// + /// Threadsafe indicator of being disposed. + /// + public bool IsDisposed + { + get + { + lock (_lock) + { + return _disposed; + } + } + } + + public void Dispose() + { + Dispose(true); + // ReSharper disable once GCSuppressFinalizeForTypeWithoutDestructor + GC.SuppressFinalize(this); + } + public required IRawJobModel RawJobModel { get; init; } public required IJobModel JobModel { get; init; } @@ -123,6 +174,7 @@ public required JobState? State /// /// Receives this entry, the original state (possibly null), and the current non-null state. /// + /// Thrown when this entry has already been disposed. public void SubscribeToState(Action action) { ArgumentNullException.ThrowIfNull(action); @@ -130,6 +182,11 @@ public void SubscribeToState(Action ac JobState? current; lock (_lock) { + if (_disposed) + { + throw new ObjectDisposedException(nameof(JobRepositoryEntry)); + } + _stateCallbacks += action; current = State; } diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index c53429a6..e08dd000 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -152,27 +152,47 @@ private void NotifyWatchedJobsUpdate(int count) private async Task TryGetUnblockedJobAsync(CancellationToken cancellationToken) { - if (!_unblockedJobsQueue.TryDequeue(out var result)) + IJobRepositoryEntry? result; + var iterated = false; + + while (_unblockedJobsQueue.TryDequeue(out result)) { - return new TryGetJobResponse + iterated = true; + // Handle potential edge case of something disposing an item that was recently unblocked + // This absolutely should not happen, but at least it won't mess things up further if it does. + + if (!result.IsDisposed) { - Success = false, - Result = null - }; + break; + } } - await _inactiveJobsListSemaphore.WaitAsync(cancellationToken); - try + if (iterated) { - if (_inactiveJobsList.Count == 0 && _unblockedJobsQueue.IsEmpty) + // Check to see if we emptied the queue, but only if we actually dequeued something + // Assume that the event is up to date and doesn't need a redundant reset. + await _inactiveJobsListSemaphore.WaitAsync(cancellationToken); + try { - // Jobs are no longer available - _jobsAvailableEvent.Reset(); + if (_inactiveJobsList.Count == 0 && _unblockedJobsQueue.IsEmpty) + { + // Jobs are no longer available + _jobsAvailableEvent.Reset(); + } + } + finally + { + _inactiveJobsListSemaphore.Release(); } } - finally + + if (result is null) { - _inactiveJobsListSemaphore.Release(); + return new TryGetJobResponse + { + Success = false, + Result = null + }; } return new TryGetJobResponse @@ -497,9 +517,23 @@ public async Task LoadAsync(IReadOnlyList intakeItems, public async Task RemoveJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default) { - // Confirm that the job that we're removing is marked as complete, - // for the sake of subscriber callbacks in the underlying JobRepositoryEntry. - job.State = JobState.Complete; + job.Dispose(); + + await _inactiveJobsListSemaphore.WaitAsync(cancellationToken); + try + { + if (_inactiveJobsList.Remove(job) + && _inactiveJobsList.Count == 0 + && _unblockedJobsQueue.IsEmpty) + { + // Jobs are no longer available + _jobsAvailableEvent.Reset(); + } + } + finally + { + _inactiveJobsListSemaphore.Release(); + } await _watchedJobsListSemaphore.WaitAsync(cancellationToken); try diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs index 784eb219..7f9608f7 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Models/JobRepositoryEntryTests.cs @@ -59,6 +59,25 @@ public async Task ConcurrentReadsAndWrites_DoNotThrow() Assert.False(jre.CanHeartbeat); } + [Fact] + public void Dispose_SetsStateToCompleteAndClearsFurtherSubscriptions() + { + var jre = CreateEntry(); + var transitions = new List<(JobState? Original, JobState Current)>(); + jre.SubscribeToState((_, original, current) => transitions.Add((original, current))); + transitions.Clear(); + + jre.Dispose(); + jre.Dispose(); + + Assert.True(jre.IsDisposed); + Assert.Equal(JobState.Complete, jre.State); + Assert.Equal([(JobState.Inactive, JobState.Complete)], transitions); + + var ex = Assert.Throws(() => jre.SubscribeToState((_, _, _) => { })); + Assert.Equal(nameof(JobRepositoryEntry), ex.ObjectName); + } + [Fact] public void State_WhenSetNull_ThrowsArgumentNullException() { @@ -141,5 +160,6 @@ public void TestGettersSetters() // Set/Get CanHeartbeat (false only) jre.CanHeartbeat = false; Assert.False(jre.CanHeartbeat); + Assert.False(jre.IsDisposed); } } \ No newline at end of file diff --git a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs index f1a1dafb..e9098ca6 100644 --- a/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs +++ b/test/RedShirt.Example.JobWorker.Core.UnitTests/Tests/Services/Jobs/JobRepositoryTests.cs @@ -32,6 +32,97 @@ private static JobRepository CreateRepository( Options.Create(new JobRepository.ConfigurationModel {BacklogSize = backlogSize})); } + private static Mock CreateJobModel(string messageId) + { + var jobModel = new Mock(MockBehavior.Strict); + jobModel.Setup(m => m.MessageId).Returns(messageId); + return jobModel; + } + + [Fact(Timeout = 2000)] + public async Task GetNextJobAsync_WhenOnlyUnblockedJobIsDisposed_FallsBackToInactiveQueue() + { + var jobRepository = CreateRepository(); + + await jobRepository.LoadAsync( + [ + new JobEnvelope + { + JobModel = CreateJobModel("unblocked-then-disposed").Object, + RawJobModel = new Mock(MockBehavior.Strict).Object + }, + new JobEnvelope + { + JobModel = CreateJobModel("still-inactive").Object, + RawJobModel = new Mock(MockBehavior.Strict).Object + } + ], + TestContext.Current.CancellationToken); + + var unblockedEntry = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); + Assert.NotNull(unblockedEntry); + unblockedEntry.State = JobState.BlockedByIdempotency; + unblockedEntry.State = JobState.Inactive; + + await jobRepository.RemoveJobAsync(unblockedEntry, TestContext.Current.CancellationToken); + Assert.True(unblockedEntry.IsDisposed); + + var nextJob = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); + + Assert.NotSame(unblockedEntry, nextJob); + Assert.NotNull(nextJob); + Assert.False(nextJob.IsDisposed); + Assert.Equal("still-inactive", nextJob.JobModel.MessageId); + Assert.Equal(JobState.Active, nextJob.State); + } + + [Fact(Timeout = 2000)] + public async Task GetNextJobAsync_WhenUnblockedJobIsDisposed_SkipsToNextUnblockedJob() + { + var jobRepository = CreateRepository(); + + await jobRepository.LoadAsync( + [ + new JobEnvelope + { + JobModel = CreateJobModel("first-unblocked").Object, + RawJobModel = new Mock(MockBehavior.Strict).Object + }, + new JobEnvelope + { + JobModel = CreateJobModel("second-unblocked").Object, + RawJobModel = new Mock(MockBehavior.Strict).Object + }, + new JobEnvelope + { + JobModel = CreateJobModel("inactive").Object, + RawJobModel = new Mock(MockBehavior.Strict).Object + } + ], + TestContext.Current.CancellationToken); + + var firstUnblocked = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); + var secondUnblocked = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); + Assert.NotNull(firstUnblocked); + Assert.NotNull(secondUnblocked); + + firstUnblocked.State = JobState.BlockedByIdempotency; + firstUnblocked.State = JobState.Inactive; + secondUnblocked.State = JobState.BlockedByIdempotency; + secondUnblocked.State = JobState.Inactive; + + await jobRepository.RemoveJobAsync(firstUnblocked, TestContext.Current.CancellationToken); + Assert.True(firstUnblocked.IsDisposed); + + var nextJob = await jobRepository.GetNextJobAsync(TestContext.Current.CancellationToken); + + Assert.NotNull(nextJob); + Assert.Same(secondUnblocked, nextJob); + Assert.False(nextJob.IsDisposed); + Assert.Equal(JobState.Active, nextJob.State); + Assert.Equal("second-unblocked", nextJob.JobModel.MessageId); + } + [Fact(Timeout = 500)] public async Task LoadAsync_WhenResponseHasNoItems_DoesNotTouchWatchedJobs() { @@ -80,8 +171,10 @@ await jobRepository.LoadAsync( Assert.Equal([0, 1, 1], watchedCounts); var job = Assert.Single(jobRepository.WatchedJobs); + Assert.False(job.IsDisposed); await jobRepository.RemoveJobAsync(job, TestContext.Current.CancellationToken); Assert.Equal(JobState.Complete, job.State); + Assert.True(job.IsDisposed); Assert.Equal([0, 1, 0], inactiveCounts); Assert.Equal([0, 1, 1, 0], watchedCounts); @@ -1038,6 +1131,7 @@ public async Task TestRemoveJobsAsync() var job = new Mock(); job.SetupProperty(j => j.State, JobState.Active); + job.Setup(j => j.Dispose()).Callback(() => job.Object.State = JobState.Complete); jobRepository.WatchedJobs.Add(job.Object); Assert.Equal(0, await jobRepository.GetInactiveJobCountAsync(TestContext.Current.CancellationToken)); @@ -1045,6 +1139,7 @@ public async Task TestRemoveJobsAsync() await jobRepository.RemoveJobAsync(job.Object, TestContext.Current.CancellationToken); + job.Verify(j => j.Dispose(), Times.Once); Assert.Equal(JobState.Complete, job.Object.State); Assert.Equal(0, await jobRepository.GetWatchedJobsCountAsync(TestContext.Current.CancellationToken)); } @@ -1074,10 +1169,12 @@ public async Task TestRemoveJobsAsyncB() var job = new Mock(); job.SetupProperty(j => j.State, JobState.Active); + job.Setup(j => j.Dispose()).Callback(() => job.Object.State = JobState.Complete); jobRepository.WatchedJobs.Add(job.Object); var job2 = new Mock(); job2.SetupProperty(j => j.State, JobState.Active); + job2.Setup(j => j.Dispose()).Callback(() => job2.Object.State = JobState.Complete); jobRepository.WatchedJobs.Add(job2.Object); Assert.Equal(0, await jobRepository.GetInactiveJobCountAsync(TestContext.Current.CancellationToken)); @@ -1085,6 +1182,8 @@ public async Task TestRemoveJobsAsyncB() await jobRepository.RemoveJobAsync(job.Object, TestContext.Current.CancellationToken); + job.Verify(j => j.Dispose(), Times.Once); + job2.Verify(j => j.Dispose(), Times.Never); Assert.Equal(JobState.Complete, job.Object.State); Assert.Equal(JobState.Active, job2.Object.State); Assert.Equal(1, await jobRepository.GetWatchedJobsCountAsync(TestContext.Current.CancellationToken)); From e1a5dab0f9752f12308a83f0b1aa1527f85eb21d Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 11:06:29 -0700 Subject: [PATCH 19/21] invert dispose order --- .../Services/Jobs/JobRepository.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index e08dd000..89367ebe 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -186,7 +186,9 @@ private async Task TryGetUnblockedJobAsync(CancellationToken } } - if (result is null) + if (result is null + // Account for technical race condition, will never happen in practice + || result.IsDisposed) { return new TryGetJobResponse { @@ -517,7 +519,7 @@ public async Task LoadAsync(IReadOnlyList intakeItems, public async Task RemoveJobAsync(IJobRepositoryEntry job, CancellationToken cancellationToken = default) { - job.Dispose(); + ArgumentNullException.ThrowIfNull(job); await _inactiveJobsListSemaphore.WaitAsync(cancellationToken); try @@ -554,6 +556,8 @@ public async Task RemoveJobAsync(IJobRepositoryEntry job, CancellationToken canc { _watchedJobsListSemaphore.Release(); } + + job.Dispose(); } public void SubscribeToInactiveCountUpdate(Action callback) From 84e3dee3cfbb343295e6adcb6ff637a19439e23c Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 11:17:04 -0700 Subject: [PATCH 20/21] minor optimizations --- .../Services/Jobs/JobRepository.cs | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index 89367ebe..a90a976b 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -393,9 +393,15 @@ public async Task GetWatchedJobsCountAsync(CancellationToken cancellationTo { await _watchedJobsListSemaphore.WaitAsync(cancellationToken); - var count = WatchedJobs.Count; - - _watchedJobsListSemaphore.Release(); + int count; + try + { + count = WatchedJobs.Count; + } + finally + { + _watchedJobsListSemaphore.Release(); + } return count; } @@ -621,11 +627,17 @@ public Task WaitForJobDemandAsync(TimeSpan waitDuration, CancellationToken public async Task WaitForEmptyRepositoryAsync(CancellationToken cancellationToken = default) { - while (await GetWatchedJobsCountAsync(cancellationToken) > 0) + int count; + bool waitResult; + do { // Short timeout mirrors GetNextJobAsync: avoids missing a Set/Reset edge under concurrency - await _repositoryEmptyEvent.WaitAsync(TimeSpan.FromMilliseconds(250), cancellationToken); - } + waitResult = await _repositoryEmptyEvent.WaitAsync(TimeSpan.FromMilliseconds(250), cancellationToken); + lock (_tallyLock) + { + count = _watchedJobsTally; + } + } while (!waitResult || count > 0); } public int GetBacklogMaxCount() From 4e8f4b863f3708bc69bf8e9e32eda332b49139e2 Mon Sep 17 00:00:00 2001 From: Alan Deutscher Date: Sat, 22 Aug 2026 11:28:30 -0700 Subject: [PATCH 21/21] just-in-case --- .../Services/Jobs/JobRepository.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs index a90a976b..c5208655 100644 --- a/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs +++ b/src/RedShirt.Example.JobWorker.Core/Services/Jobs/JobRepository.cs @@ -628,16 +628,15 @@ public Task WaitForJobDemandAsync(TimeSpan waitDuration, CancellationToken public async Task WaitForEmptyRepositoryAsync(CancellationToken cancellationToken = default) { int count; - bool waitResult; do { // Short timeout mirrors GetNextJobAsync: avoids missing a Set/Reset edge under concurrency - waitResult = await _repositoryEmptyEvent.WaitAsync(TimeSpan.FromMilliseconds(250), cancellationToken); + await _repositoryEmptyEvent.WaitAsync(TimeSpan.FromMilliseconds(250), cancellationToken); lock (_tallyLock) { count = _watchedJobsTally; } - } while (!waitResult || count > 0); + } while (count > 0); } public int GetBacklogMaxCount()