BrighterCommand / BrighterCommand/Brighter

Parallelise per-phase BoxProvisioningHostedService for multi-tenant deployments

Open
#4,140 1 comment 0 reactions 0 assignees View on GitHub
.NET 0 - Backlog feature request under_discussion V11
Dominant language
C#
Stars
2.5k
Forks
296
Avg merge
1d 11h
Merged PRs (30d)
21

Description

## Context

PR #4039's `BoxProvisioningHostedService.StartAsync` provisions all
registered boxes sequentially in a single `foreach`:

```csharp
// src/Paramore.Brighter.BoxProvisioning/BoxProvisioningHostedService.cs:82-90
var ordered = _provisioners.OrderBy(p => p.BoxType); // Outbox before Inbox
foreach (var provisioner in ordered)
{
await provisioner.ProvisionAsync(cancellationToken);
}
```

For a single-database deployment this is correct (the database-level
migration lock would serialise concurrent provisioners anyway).
Multi-tenant deployments with N independent databases pay a linear
startup cost: total time = Σ per-tenant migration time, where
ideally it should be max(per-tenant migration time).

## What needs doing

Group provisioners by `BoxType` and run each phase with
`Task.WhenAll`, preserving the Outbox-before-Inbox ordering required
by the API contract:

```csharp
var byPhase = _provisioners.GroupBy(p => p.BoxType).OrderBy(g => g.Key);
foreach (var phase in byPhase)
{
await Task.WhenAll(phase.Select(p => p.ProvisionAsync(cancellationToken)));
}
```

Considerations:
- Per-tenant migration locks remain per-database, so parallelism does
not affect lock semantics.
- Exception aggregation: `Task.WhenAll` aggregates faulted tasks into
a single `AggregateException`. The current sequential code fails
fast on the first throw. Decide whether to preserve fail-fast (use
`Task.WhenEach` or a custom wait pattern) or accept aggregation.
- Cancellation propagation: each `ProvisionAsync` already accepts a
shared `CancellationToken`; no change needed.
- Logging shape: the existing per-provisioner Information logs will
interleave; consider whether to keep that or wrap each Task with a
scoped logger.

## Why this is a follow-up, not part of PR #4039

The reviewer flagged this as observational ("Worth deferring unless
multi-tenant scenarios surface as a real concern"). No deployment
in-scope for PR #4039 exercises the multi-DB path.

## Acceptance

- Per-phase parallel provisioning behind `Task.WhenAll`.
- Tests covering: (a) parallel exec preserves Outbox-before-Inbox
ordering, (b) exception in one provisioner does not prevent siblings
from finishing (or, if fail-fast preferred, that the first failure
cancels siblings).
- No regression on single-database deployments.

## Reference

PR #4039 review comment:
https://github.com/BrighterCommand/Brighter/pull/4039#issuecomment-4480741284

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Contributor guide

Open the contributing guide

Research direction

Start in src/Paramore.Brighter.BoxProvisioning/BoxProvisioningHostedService.cs and review PR #4039's provisioning changes. Trace BoxType ordering, per-tenant ProvisionAsync calls, cancellation, and the existing provisioning tests before deciding how failures should behave. Done means per-phase provisioning is parallel, Outbox still precedes Inbox, and the acceptance cases pass without single-database regression.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend, database
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.