From 0135f114d34bc3b3604db7a5c5cbc1c5a0164c8d Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 23:40:33 +0200 Subject: [PATCH] fix(traffic): keep retrying the throttle release after a Proxmox blip Tying the release to the creation of the new month's row meant a single failed call was never retried: the row exists from then on, and the current period looks unthrottled, so nothing could detect the stale NIC limit. A customer would have stayed slow for a whole month they had paid for. Checked on every sample now, with a test that makes the first release fail. Co-Authored-By: Claude Opus 4.8 --- .../Jobs/CollectInstanceTraffic.php | 12 +++--- tests/Feature/TrafficTest.php | 39 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/app/Provisioning/Jobs/CollectInstanceTraffic.php b/app/Provisioning/Jobs/CollectInstanceTraffic.php index 59de491..7839bb6 100644 --- a/app/Provisioning/Jobs/CollectInstanceTraffic.php +++ b/app/Provisioning/Jobs/CollectInstanceTraffic.php @@ -74,12 +74,12 @@ class CollectInstanceTraffic implements ShouldBeUnique, ShouldQueue ['last_netin' => $netin, 'last_netout' => $netout, 'sampled_at' => now()], ); - // The month rolled over: the new row starts unthrottled, but the NIC - // still carries last month's limit and nothing below would ever take it - // off — a customer would stay slow into a month they have paid for. - if ($row->wasRecentlyCreated) { - $this->releasePreviousPeriodThrottle($client, $instance); - } + // Checked on every sample, not only when the row was just created: if + // the release failed once — a Proxmox blip during the rollover run — + // tying it to row creation would never try again, and the customer would + // stay slow for a month they have paid for. The lookup exits + // immediately when there is nothing to release. + $this->releasePreviousPeriodThrottle($client, $instance); // A fresh row starts from this sample: whatever the VM transferred // before we started counting this month is not ours to bill. diff --git a/tests/Feature/TrafficTest.php b/tests/Feature/TrafficTest.php index 24ea9f9..e9de2da 100644 --- a/tests/Feature/TrafficTest.php +++ b/tests/Feature/TrafficTest.php @@ -179,3 +179,42 @@ it('lets a throttled instance run again when the new month starts', function () expect($pve->networkRates[1042])->toBeNull() ->and(InstanceTraffic::query()->where('throttled', true)->count())->toBe(0); }); + +it('keeps trying to release a throttle when Proxmox refuses once', function () { + [$pve, , $instance] = trafficSetup(); + + InstanceTraffic::create([ + 'instance_id' => $instance->id, + 'period' => now()->subMonth()->format('Y-m'), + 'tx_bytes' => 9_999_999_999_999, + 'throttled' => true, + ]); + + // A hub that fails the first release and works afterwards — the transient + // error that would otherwise leave the customer throttled for good. + $flaky = new class extends FakeProxmoxClient + { + public int $attempts = 0; + + public function setNetworkRate(string $node, int $vmid, ?float $mbytesPerSecond): void + { + $this->attempts++; + if ($mbytesPerSecond === null && $this->attempts === 1) { + throw new RuntimeException('proxmox unavailable'); + } + parent::setNetworkRate($node, $vmid, $mbytesPerSecond); + } + }; + $flaky->counters[1042] = ['netin' => 0, 'netout' => 0]; + app()->instance(ProxmoxClient::class, $flaky); + + (new CollectInstanceTraffic)->handle($flaky); + expect(InstanceTraffic::query()->where('throttled', true)->count())->toBe(1); + + // Next run: the current row already exists, so a release tied to row + // creation would never fire again. + (new CollectInstanceTraffic)->handle($flaky); + + expect(InstanceTraffic::query()->where('throttled', true)->count())->toBe(0) + ->and($flaky->networkRates[1042])->toBeNull(); +});