Batch example#175
Conversation
Processes a batch by having a single Activity heartbeat its progress, so a worker restart resumes from the last recorded offset instead of starting over. Mirrors samples-java/.../batch/heartbeatingactivity.
Processes a page of records at a time via child workflows run in parallel, continuing-as-new between pages so a dataset of any size can be processed. Mirrors samples-java/.../batch/iterator.
Keeps a fixed number of record processing child workflows running in parallel at all times, using completion Signals and continue-as-new to keep history bounded indefinitely. Multiple partitions run side by side via BatchWorkflow for higher total throughput. Continues-as-new on Workflow.ContinueAsNewSuggested in addition to a fixed page size, matching StreamPay's ProcessCustomerDataWorkflow, since a fixed count alone can be outgrown by other history (e.g. completion Signals) before it's reached. A TestContinueAsNew input flag makes that branch deterministically testable. Mirrors samples-java/.../batch/slidingwindow.
Adds the Batch solution folder and its three project references to TemporalioSamples.sln and their test-project references to TemporalioSamples.Tests.csproj, lists Batch in the root README's sample index, and adds the overview README for src/Batch.
jmaeagle99
left a comment
There was a problem hiding this comment.
Thanks for contributing these samples. Wanted to provide some initial feedback while I go through the rest of it.
| public static IReadOnlyList<SingleRecord> GetRecords(int pageSize, int offset) | ||
| { | ||
| var records = new List<SingleRecord>(pageSize); | ||
| if (offset < pageSize * PageCount) |
There was a problem hiding this comment.
This sample set size being a function of the pageSize is slightly odd. I would suggest changing the PageCount const to a TotalCount and just compare offset to TotalCount, just like you did in SlidingWindow.
| () => RecordLoaderActivities.GetRecordCount(), | ||
| new() { StartToCloseTimeout = TimeSpan.FromSeconds(5) }); | ||
|
|
||
| var partitionSize = (totalCount / partitions) + (totalCount % partitions > 0 ? 1 : 0); |
There was a problem hiding this comment.
nit: this looks like ceiling division, which can lead to the last partition being empty depending on the inputs. So it might start a child workflow that does nothing. Not necessarily a bug but an unnecessary execution.
For example, totalCount = 5 and partitions = 4 gives a partitionSize of 2. Those partitions are [0, 2), [2, 4], [4, 5), and [6, 5). The last one is clearly empty.
| /// reporting completion: a signal can be delivered to the new run before its workflow run | ||
| /// method has had a chance to populate <see cref="currentRecords"/> from the input. | ||
| /// </summary> | ||
| private readonly HashSet<int> recordsToRemove = new(); |
There was a problem hiding this comment.
Take a look at WorkflowInit, which would allow defining a constructor with the same arguments as the WorkflowRun method. Might be able to set currentRecords from the constructor and have the signal handler and run methods use that field without having to store extra state.
|
|
||
| // For ease of testing, forces continue-as-new well before the real history-size | ||
| // threshold would suggest it, instead of relying on Workflow.ContinueAsNewSuggested | ||
| // alone. Mirrors ClusterManagerWorkflow's maxHistoryLength in the SafeMessageHandlers |
There was a problem hiding this comment.
Please remove the reference to the SafeMessageHandlers sample. It doesn't matter what the other samples do, and it's quite possible they will naturally diverge over time. Leaving this type of comment might cause confusion if that divergence occurs.
|
|
||
| These are .NET ports of the | ||
| [Java batch samples](https://github.com/temporalio/samples-java/tree/main/core/src/main/java/io/temporal/samples/batch), | ||
| kept as close as possible to the original Java structure and naming. |
There was a problem hiding this comment.
Please remove "kept as close as possible to the original Java structure and naming". That shouldn't bear weight on the .NET samples. The samples can diverge over time and this line is nearly promising that they will not.
| Multiple instances of `SlidingWindowBatchWorkflow` run in parallel, each processing a subset of | ||
| records, to support a higher total rate of processing. | ||
|
|
||
| This is the sample that demonstrates the workaround for the race between continue-as-new and a |
There was a problem hiding this comment.
This set of comments will need to be updated if the WorkflowInit feature is employed.
jmaeagle99
left a comment
There was a problem hiding this comment.
Also, in the PR description, you mention "which came up in a community discussion about batch-processing workarounds". Do you have a link to the community discussion that you could share? Or you could drop it into the Temporal Community dotnet-sdk Slack channel.
What was changed
Added a
.NETport of the Javabatchsamples (samples-java/core/src/main/java/io/temporal/samples/batch), under a newsrc/Batchdirectory with one project per pattern:BatchWorkflowfor higher total throughput.Each sample follows this repo's existing conventions (
dotnet run worker/dotnet run workflow, per-sample README,TemporalioSamples.slnentries) and mirrors the Java sample's file/class structure closely so it's easy to compare side by side.One deliberate deviation from the Java version:
SlidingWindowBatchWorkflowcontinues-as-new onWorkflow.ContinueAsNewSuggestedin addition to a fixed page size, rather than relying on the fixed count alone. A fixed count can be outgrown by other accumulated history (e.g. completion Signals) before it's reached — this pattern is based on how we handle the same problem in production. ATestContinueAsNewinput flag makes that branch deterministically testable without generating a large amount of real history.Why?
There was no .NET equivalent of these samples, which came up in a community discussion about batch-processing workarounds (paging through large datasets, bounding history size, safely reporting child-Workflow completion across continue-as-new). This fills that gap using patterns we've validated in a production import pipeline.
Checklist
Closes [Sample Request] Batch #131
How was this tested:
dotnet build/dotnet format --verify-no-changeson the full solution.tests/Batch/*(xUnit, usingWorkflowEnvironment.StartTimeSkippingAsyncfor the Iterator and SlidingWindow Workflow tests, andActivityEnvironmentfor a targeted heartbeat-resume test on HeartbeatingActivity). All pass.temporalCLI. This caught and fixed a real bug during development:SlidingWindowBatchWorkflow's original return value summed to double the actual record count across partitions; it now returns the Signal-trackedprogresscount instead.Any docs updates needed?
src/Batch/README.md(overview) plus a README per sample.Batchentry to the rootREADME.mdsample index.