Add CPU billing #454
Add CPU billing #454
Conversation
- Instead of a thread per transaction, we use a single global thread - The timer is now based on CPU time rather than real time
…cuting any non-privileged code
|
|
||
| ## Clocks | ||
|
|
||
| UTC, monotonic, and CPU clocks are available to subjective services. The monotonic clock is only guaranteed to be monotonic within a block. The CPU clock measures CPU time spent on the current transaction. |
There was a problem hiding this comment.
This should define or link to something that defines "subjective services"
There was a problem hiding this comment.
I can't find any existing documentation for subjective services and it certainly doesn't belong here. Maybe under services/concepts?
There was a problem hiding this comment.
Perhaps just in services itself
There was a problem hiding this comment.
I think it should be deferred to another PR. Documentation of subjective services is only incidentally related and this PR is already quite large.
| @@ -57,7 +61,8 @@ namespace psibase | |||
| std::lock_guard<std::mutex> lock{impl->mutex}; | |||
| if (impl->systemContextCache.empty()) | |||
| { | |||
| auto result = std::make_unique<SystemContext>(SystemContext{impl->db, impl->wasmCache}); | |||
| auto result = std::make_unique<SystemContext>( | |||
| SystemContext{impl->db, impl->wasmCache, {}, impl->watchdogManager}); | |||
| impl->allSystemContexts.push_back(result.get()); | |||
There was a problem hiding this comment.
This looks sketchy. Couldn't this lead to UB if the unique ptr is destroyed and the linearMemorySpan is called?
There was a problem hiding this comment.
Yes, though that never happens. Also, note the comment on the declaration of allSystemContexts.
There was a problem hiding this comment.
This is pre-existing code that is not modified by this PR.
| std::chrono::nanoseconds getCpuTime(); | ||
| void setCpuLimit(psibase::AccountNumber account); | ||
| }; | ||
| PSIO_REFLECT(CpuSys, method(getCpuTime), method(setCpuLimit, limit)) |
There was a problem hiding this comment.
Should be method(setCpuLimit, account)
| @@ -293,6 +294,9 @@ namespace SystemService | |||
| "transaction references non-existing block"); | |||
| } | |||
|
|
|||
| Actor<CpuSys> cpuSys(TransactionSys::service, CpuSys::service); | |||
| Actor<AccountSys> accountSys(TransactionSys::service, AccountSys::service); | |||
There was a problem hiding this comment.
Do you not prefer the to<Service>.action syntax? I like how it reads
There was a problem hiding this comment.
to doesn't work here, because it uses state set up by the dispatcher. processTransaction is a direct export that doesn't go through the dispatcher.
| @@ -606,7 +604,7 @@ bool pushTransaction(psibase::SharedState& sharedState, | |||
| // for a modified node to skip it during production. This won't hurt | |||
| // consensus since replay never uses read-only mode for auth checks. | |||
| auto saveTrace = trace; | |||
| proofBC.checkFirstAuth(trx, trace, firstAuthWatchdogLimit); | |||
| proofBC.checkFirstAuth(trx, trace, std::nullopt); | |||
There was a problem hiding this comment.
Why wouldn't we default the watchdog limit here to the leeway?
There was a problem hiding this comment.
I want to make the leeway option go away entirely and have it managed entirely on chain.
James-Mart
added
the
Node infra
By default, accounts have unlimited resources. The billing system is minimal—just enough to ensure that the host functions are sufficient and work correctly.
This also allows boot to complete reliably without timing out, as timeouts are disabled until boot is completed.