Skip to content

fix: load the current charged event counts after a container restart - #1218

Closed
Pijukatel wants to merge 18 commits into
masterfrom
claude/focused-archimedes-yoql5b
Closed

Pijukatel wants to merge 18 commits into
masterfrom
claude/focused-archimedes-yoql5b

Conversation

@Pijukatel

@Pijukatel Pijukatel commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

After a reboot, migration or restart, the platform seeds the new container with the charged event counts recorded when the run started, so the charging manager began from a fresh max_total_charge_usd budget. The SDK now reads the run before its first charge and continues from what was already charged.

  • On a platform pay-per-event run with at least one event the SDK can charge, the charge lock reads the run the first time it is taken, which is before the first charge() or budgeted push_data() and updates the charge info from API.
  • Reading late and lazily as opposed to eager reading on init gives the platform time to report correct values that can lag due to eventual consistency. Also there is no pre-emptive API call if no charging is used.
  • Runs without SDK-chargeable events, non-PPE runs and local runs make no extra request. A failed read is logged and the env var counts are kept.
  • The synchronous read methods report the start-of-run counts until that first charge or push. Making them async, so they trigger the load too would be a breaking change. (Can be potentially done for the next major release.)

Closes: #1209

claude and others added 4 commits October 30, 2026 09:24
After a reboot, migration or restart the platform seeds the new container
with the `APIFY_CHARGED_ACTOR_EVENT_COUNTS` recorded when the run started,
so the charging manager began from zero charged events and allowed a fresh
`max_total_charge_usd` budget.

The charging manager now persists its charged event counts to the default
key-value store under `__ACTOR_CHARGING_STATE` on every `PERSIST_STATE`
event and right before `Actor.reboot()` calls the API, writing only when a
charge happened since the last write. A container that starts more than
60 seconds after the run started reads the record and takes the persisted
count of each event where it is higher than the platform's. A prompt first
start makes no extra request. A record of another run, a malformed one, or
a failed read is logged and ignored.

Closes #1209

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
…them

Replaces the key-value store persistence with a read of the run through
the API, which needs no writes during the run, no new record, and no
reboot hook, and also covers crash restarts.

A pay-per-event run on the platform whose pricing has at least one event
the SDK can charge, and whose container starts more than 60 seconds after
the run started, reads the run and takes the current count of each event
where it is higher than the env var's. A prompt first start, a run without
SDK-chargeable events, and a local run make no extra request. A failed
read is logged and the env var's counts are kept.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
…tart

A reboot restarts the container in about a second, so a restart cannot
be told apart from a first start by the time elapsed since the run
started. The time threshold is dropped, and every platform run with a
pay-per-event pricing that has at least one event the SDK can charge
reads the run at start and takes the current count of each event where
it is higher than the env var's. Runs without SDK-chargeable events and
local runs make no extra request.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
@github-actions github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Oct 30, 2026
@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.43%. Comparing base (4556821) to head (47c2926).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/apify/_charging.py 94.87% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1218      +/-   ##
==========================================
+ Coverage   93.42%   93.43%   +0.01%     
==========================================
  Files          54       54              
  Lines        3771     3809      +38     
==========================================
+ Hits         3523     3559      +36     
- Misses        248      250       +2     
Flag Coverage Δ
e2e 31.39% <23.07%> (-0.08%) ⬇️
integration 51.14% <33.33%> (-0.20%) ⬇️
unit 90.31% <94.87%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

claude and others added 5 commits October 30, 2026 13:54
The run is read before the first charge or budgeted push of a container
instead of at init, so the platform has time to record the last charges
of a previous container, and a run that never charges makes no request.
Until then, the synchronous methods report the counts the run started
with.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
`_fetch_pricing_info` reports whether the counts were read from the run,
and `__aenter__` records it, so the loader has no side effect on the
manager. The chargeable-run check uses the pricing already loaded.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
@Pijukatel
Pijukatel force-pushed the claude/focused-archimedes-yoql5b branch from b0a6cd8 to 805e5fd Compare October 30, 2026 14:51
…ed counts

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
The charge lock brings the counts up to date the first time it is taken,
so neither `charge` nor the push path needs to know about the refresh.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAKQTqReMDTjXCUTnKP

@apify-factory apify-factory left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one note.

🏭 Factory review — ℹ️ not auto-approved

This change was reviewed but cannot be auto-approved: a gate failed, so the review cannot end in an approval.

check result
Static gates ❌ 14/16 — pr-open-and-ready, deny-globs
Review findings ❌ not clean
Reviewer escalation ❌ Changes pay-per-event charging limit accounting (money) and adds an API request to the charge path on platform runs.
Juror 1 — claude-sonnet-5-5, adversarial ⏭️ skipped — a gate failed
Why it was not auto-approved

Failed gates:

  • pr-open-and-ready: PR is a draft
  • deny-globs: src/apify/_charging.py matches src/apify/_charging.py

The comments above are the review itself; this section only explains the approval decision. Push a change and request @apify-factory again to re-run it, or ask a human to review.

Reviewed 896e79144 · workflow run

Comment thread src/apify/_charging.py Resolved
@Pijukatel Pijukatel added the adhoc Ad-hoc unplanned task added during the sprint. label Oct 30, 2026
@Pijukatel
Pijukatel requested a review from vdusek October 30, 2026 15:30
@Pijukatel
Pijukatel marked this pull request as ready for review October 30, 2026 15:30
@apify-service-account apify-service-account added the tested Temporary label used only programatically for some analytics. label Oct 30, 2026
@Pijukatel
Pijukatel marked this pull request as draft October 30, 2026 15:40
@Pijukatel

Copy link
Copy Markdown
Contributor Author

Hmm, platform's eventual consistency is too slow for this again :( , probably we will have to track it again ourselves

@Pijukatel Pijukatel closed this Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ChargingManager reads charged counts and total as 0 after Actor.reboot(), so maxTotalChargeUsd allows a fresh budget

4 participants