RomanNum3ral
/
whisper-money
mirror of https://github.com/whisper-money/whisper-money.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(balances): stop historical-balance generator OOM on ancient purchase/loan dates (PHP-LARAVEL-49) (#661) 

## What & why

A queued historical-balance generator job exhausted the worker's 128MB
PHP memory limit and **died on every retry** — Sentry `PHP-LARAVEL-49` +
`PHP-LARAVEL-4A` (two fingerprints of the *same* poison message:
identical `trace_id`/`message.id`, `retry.count` 1 then 2, OOM at
slightly different `Arr.php` lines).

### Root cause (trace-confirmed)
The Sentry trace localized the OOMing message to
`App\Jobs\GenerateHistoricalRealEstateBalancesJob`, dispatched from
`Settings\AccountController::store` for the "older than 12 months" slice
of history. The fatal frame is `Arr::map` inside Eloquent's
`AccountBalance::upsert()` value-prep (`addUniqueIds`/`addTimestamps`
per-row `array_merge`, `Query\Builder::upsert` `Arr::flatten` bindings,
and a compiled multi-row SQL string — all **O(n)** over the rows).

`purchase_date` (`before_or_equal:today`) and `loan_start_date` (no
constraint) had **no lower bound**, so a mistyped/ancient year (e.g.
`0201`, `1080`) made the generator build a **multi-century monthly
series** and hand it all to a single `upsert`, blowing past 128MB.

## The fix (two commits)

1. **`fix(balances): upsert historical balances in batches`** — build +
upsert in batches of 500 (`UPSERT_CHUNK_SIZE`) in both
`RealEstateBalanceGeneratorService` and `LoanBalanceGeneratorService`,
instead of one giant operation. Same rows, same `(account_id,
balance_date)` conflict target — peak memory is now bounded regardless
of series length. Defense-in-depth.
2. **`fix(accounts): floor purchase/loan start dates at 1900`** — the
actual root cause: add `after_or_equal:1900-01-01` to `purchase_date`
and `loan_start_date`. Rejects the data-entry typos that drive the
runaway series (and the resulting DB bloat / misleading multi-century
net-worth ramp) while accepting any legitimately old asset in a
personal-finance app.

## Review (two agents, before this PR)
Both reviewed against the live trace and the code.

- **Root cause & fix confirmed** — the batching caps exactly the
`Arr::map`-over-N-rows frame from the trace; no independent OOM source
exists (no per-row model events; `upsert` bypasses events).
- **Chunking is correct** — the unique index `(account_id,
balance_date)` exists, so the conflict target is real; `buildDateList`
yields a strictly-increasing de-duplicated list, so batches are disjoint
(no dropped/duplicated dates, no off-by-one). The fresh per-row `id`
UUID is discarded on conflict → idempotent retries.
- **No material user-facing regression.** Balance consumption
(dashboard/net-worth) is date-range-clamped. Batching introduces a
theoretical partial-write-on-mid-job-failure window, but it's idempotent
and self-heals on retry — agents advised **against** wrapping it in a
transaction (holds locks, doesn't help memory).

Applied recommendation #2 (the date floor) as its own commit. Deferred
(both agents agree): extracting the two near-identical generators into a
shared base/trait — a larger refactor that shouldn't ride a hotfix.

## Tests
- Regression test in each service: generate a ~46-year (>500-point)
series and assert the batched writes produce no dropped/duplicated dates
and keep endpoints anchored (crosses multiple batches).
- Validation test: a pre-1900 `purchase_date` is rejected at
`accounts.store`.

Note: the PHP feature suite boots a MySQL testcontainer (Docker) that
isn't available in my environment, so these were validated by
static/`pint`/`php -l` locally and rely on CI for execution — auto-merge
is gated on green CI.

Fixes PHP-LARAVEL-49
Fixes PHP-LARAVEL-4A
This commit is contained in:
Víctor Falcón 2026-07-08 23:34:43 +02:00 committed by GitHub
parent 1a62b8f42e
commit 08d367a5c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 144 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/Http/Requests/Concerns/ValidatesAccountDetailRules.php
View File

@ -24,7 +24,10 @@ trait ValidatesAccountDetailRules
],
'address' => ['nullable', 'string', 'max:500'],
'purchase_price' => ['nullable', 'integer', 'min:0'],
'purchase_date' => ['nullable', 'date', 'before_or_equal:today'],
// Floor the date: a mistyped/ancient year would make the historical
// balance generator build a multi-century monthly series and OOM the
// queue worker (PHP-LARAVEL-49). 1900 rejects typos, not real assets.
'purchase_date' => ['nullable', 'date', 'after_or_equal:1900-01-01', 'before_or_equal:today'],
'area_value' => ['nullable', 'numeric', 'min:0', 'max:99999999.99'],
'area_unit' => ['nullable', 'string', Rule::in(['sqm', 'sqft', 'acres', 'hectares'])],
'linked_loan_account_id' => [
@ -52,7 +55,9 @@ trait ValidatesAccountDetailRules
return [
'annual_interest_rate' => ['nullable', 'numeric', 'min:0', 'max:100'],
'loan_term_months' => ['nullable', 'integer', 'min:1', 'max:600'],
'loan_start_date' => ['nullable', 'date'],
// Floor the date for the same reason as purchase_date above: an
// ancient loan start date OOMs the balance generator (PHP-LARAVEL-49).
'loan_start_date' => ['nullable', 'date', 'after_or_equal:1900-01-01'],
'original_amount' => ['nullable', 'integer', 'min:0'],
];
}

28
app/Services/LoanBalanceGeneratorService.php
View File

@ -9,6 +9,15 @@ use Illuminate\Support\Str;
class LoanBalanceGeneratorService
{
/**
* Upsert historical balances in batches of this size. An old loan start
* date (which has no lower bound in validation) can produce a very long
* monthly series; building and upserting it all at once exhausts the queue
* worker's memory in Arr::map/flatten (PHP-LARAVEL-49). Batching bounds
* peak memory.
*/
private const UPSERT_CHUNK_SIZE = 500;
/**
* Generate historical monthly balances from a loan's start date to today
* using linear interpolation between the original amount owed and the
@ -73,6 +82,25 @@ class LoanBalanceGeneratorService
'created_at' => $now,
'updated_at' => $now,
];
if (count($rows) >= self::UPSERT_CHUNK_SIZE) {
$this->upsertBalances($rows);
$rows = [];
}
}
$this->upsertBalances($rows);
}
/**
* Upsert a batch of balance rows, keyed by (account_id, balance_date).
*
* @param list<array<string, mixed>> $rows
*/
private function upsertBalances(array $rows): void
{
if ($rows === []) {
return;
}
AccountBalance::upsert($rows, ['account_id', 'balance_date'], ['balance', 'updated_at']);

27
app/Services/RealEstateBalanceGeneratorService.php
View File

@ -9,6 +9,14 @@ use Illuminate\Support\Str;
class RealEstateBalanceGeneratorService
{
/**
* Upsert historical balances in batches of this size. An old purchase date
* (which has no lower bound in validation) can produce a very long monthly
* series; building and upserting it all at once exhausts the queue worker's
* memory in Arr::map/flatten (PHP-LARAVEL-49). Batching bounds peak memory.
*/
private const UPSERT_CHUNK_SIZE = 500;
/**
* Generate historical monthly balances from purchase date to today
* using linear interpolation between purchase price and current value.
@ -73,6 +81,25 @@ class RealEstateBalanceGeneratorService
'created_at' => $now,
'updated_at' => $now,
];
if (count($rows) >= self::UPSERT_CHUNK_SIZE) {
$this->upsertBalances($rows);
$rows = [];
}
}
$this->upsertBalances($rows);
}
/**
* Upsert a batch of balance rows, keyed by (account_id, balance_date).
*
* @param list<array<string, mixed>> $rows
*/
private function upsertBalances(array $rows): void
{
if ($rows === []) {
return;
}
AccountBalance::upsert($rows, ['account_id', 'balance_date'], ['balance', 'updated_at']);

20
tests/Feature/RealEstateTest.php
View File

@ -63,6 +63,26 @@ it('can create a real estate account with property details', function () {
]);
});
it('rejects a purchase date before the 1900 floor', function () {
actingAs($this->user);
// A pre-1900 (typically mistyped) date would make the historical balance
// generator build a multi-century monthly series and OOM the queue worker
// (PHP-LARAVEL-49). It must be rejected at validation.
$data = [
'name' => 'My Apartment',
'currency_code' => 'EUR',
'type' => AccountType::RealEstate->value,
'property_type' => PropertyType::Residential->value,
'purchase_price' => 25000000,
'purchase_date' => '1899-12-31',
];
$response = $this->post(route('accounts.store'), $data);
$response->assertSessionHasErrors(['purchase_date']);
});
it('can create a real estate account with only required fields', function () {
actingAs($this->user);

31
tests/Feature/Services/LoanBalanceGeneratorServiceTest.php
View File

@ -10,6 +10,37 @@ beforeEach(function () {
$this->service = app(LoanBalanceGeneratorService::class);
});
it('generates a long series across multiple upsert batches without gaps or duplicates', function () {
$this->travelTo(Carbon::parse('2026-06-15'));
$account = Account::factory()->loan()->create([
'user_id' => $this->user->id,
]);
// ~46 years of monthly points is well over the internal upsert batch size,
// so this drives the chunked build/upsert path that stops the queue worker
// from exhausting memory on an ancient loan start date (PHP-LARAVEL-49).
$this->service->generateHistoricalBalances(
$account,
originalAmount: 15000000,
startDate: Carbon::parse('1980-01-15'),
currentBalance: 3000000,
);
$balances = $account->balances()->orderBy('balance_date')->get();
$dates = $balances->pluck('balance_date')->map->toDateString();
// Enough points to span more than one batch...
expect($balances->count())->toBeGreaterThan(500);
// ...and the batch boundaries must not drop or duplicate any date.
expect($dates->unique()->count())->toBe($balances->count());
// Endpoints stay anchored across the batched writes.
expect($dates->first())->toBe('1980-01-15');
expect($balances->first()->balance)->toBe(15000000);
expect($dates->last())->toBe('2026-06-15');
expect($balances->last()->balance)->toBe(3000000);
});
it('generates linearly interpolated balances from start date to today', function () {
$this->travelTo(Carbon::parse('2026-03-15'));

31
tests/Feature/Services/RealEstateBalanceGeneratorServiceTest.php
View File

@ -10,6 +10,37 @@ beforeEach(function () {
$this->service = app(RealEstateBalanceGeneratorService::class);
});
it('generates a long series across multiple upsert batches without gaps or duplicates', function () {
$this->travelTo(Carbon::parse('2026-06-15'));
$account = Account::factory()->realEstate()->create([
'user_id' => $this->user->id,
]);
// ~46 years of monthly points is well over the internal upsert batch size,
// so this drives the chunked build/upsert path that stops the queue worker
// from exhausting memory on an ancient purchase date (PHP-LARAVEL-49).
$this->service->generateHistoricalBalances(
$account,
purchasePrice: 10000000,
purchaseDate: Carbon::parse('1980-01-15'),
currentValue: 16000000,
);
$balances = $account->balances()->orderBy('balance_date')->get();
$dates = $balances->pluck('balance_date')->map->toDateString();
// Enough points to span more than one batch...
expect($balances->count())->toBeGreaterThan(500);
// ...and the batch boundaries must not drop or duplicate any date.
expect($dates->unique()->count())->toBe($balances->count());
// Endpoints stay anchored across the batched writes.
expect($dates->first())->toBe('1980-01-15');
expect($balances->first()->balance)->toBe(10000000);
expect($dates->last())->toBe('2026-06-15');
expect($balances->last()->balance)->toBe(16000000);
});
it('generates linearly interpolated balances from purchase date to today', function () {
$this->travelTo(Carbon::parse('2026-03-15'));