Feature #18205
closed[api] [cloud] add compute instance price to container record
Description
Add new fields to the container record that contain:
- the total cost of a container (if the container is running, the cost up to this point in time)
- the total cost of the container and all of its child containers (for workflow runners) (if the container is running, the cost up to this point)
- the incremental cost of the process tree, where reused containers are excluded from the calculation
When the container is completed, either the dispatcher or API server should compute the cost using InstanceType.Price. Getting live cloud prices will done in #19320
Updated by Ward Vandewege about 3 years ago
- Blocks Idea #18179: Better spot instance support added
Updated by Ward Vandewege about 3 years ago
- Blocks Feature #17695: [costanalyzer] make an accurate report for spot instances on AWS added
Updated by Peter Amstutz over 2 years ago
- Target version changed from 2022-07-20 to 2022-08-17 sprint
Updated by Peter Amstutz over 2 years ago
- Target version changed from 2022-08-17 sprint to 2022-08-03 Sprint
Updated by Peter Amstutz over 2 years ago
- Target version changed from 2022-08-03 Sprint to 2022-08-17 sprint
Updated by Peter Amstutz over 2 years ago
- Blocks Feature #19319: Display container price information added
Updated by Peter Amstutz over 2 years ago
- Blocks Feature #19320: Get actual instance price information by calling AWS APIs added
Updated by Peter Amstutz over 2 years ago
- Description updated (diff)
- Subject changed from [api] [cloud] add live compute instance price to container record to [api] [cloud] add compute instance price to container record
Updated by Tom Clegg over 2 years ago
18205-container-cost @ f5512fd7f9abe306740af464551938461033a935 -- developer-run-tests: #3262
... retry wb1 developer-run-tests-apps-workbench-integration: #3508
... retry wb1 developer-run-tests-apps-workbench-integration: #3509
(different wb1 failures each time, sigh)
New fields:- containers.cost: cost of the container itself
- containers.subrequests_cost: total cumulative cost of all container requests submitted by this container
- container_requests.cumulative_cost: total cost of containers attributable to this request (including cumulative cost of subrequests) excluding containers that were reused, i.e., had already completed when this request was committed.
We might want to tweak the conditions for excluding reused container costs (e.g., currently if four concurrent requests all use the same container, the cost will be added to all four of them -- perhaps it should only be added to one, or split among the four). At this point I'm aiming to get the answer right in the most common/obvious cases, e.g., a workflow run that has some brand new containers and some that were reused from previous runs, so we can unblock #19319.
The cost estimates will not be great when using preemptible instances, since we don't yet ask cloud providers for real price data (#19320).
Updated by Lucas Di Pentima over 2 years ago
Sorry for the delay, I'll review this tomorrow first thing in the morning.
Updated by Lucas Di Pentima over 2 years ago
Just one question:
- Should we use the "money" type for these new fields? Floats aren't recommended because of rounding errors: https://www.postgresql.org/docs/10/datatype-money.html
The rest LGTM, thanks.
Updated by Tom Clegg over 2 years ago
I considered using a fixed-point type, but they have worse performance characteristics (for sort/filter). The "money" type has the additional "locale-sensitive" caveat: "To avoid problems, before restoring a dump into a new database make sure lc_monetary has the same or equivalent value as in the database that was dumped."
I figure since these numbers are inherently imprecise anyway (there are several reasons why they will never agree exactly with real billing figures) we wouldn't really benefit by rounding to a specified number of decimal places at each database interaction.
WDYT?
Updated by Peter Amstutz over 2 years ago
- Target version changed from 2022-08-17 sprint to 2022-08-31 sprint
Updated by Lucas Di Pentima over 2 years ago
Sorry for the delay, didn't got the notification on this comment.
I agree with your comments, LGTM.
Updated by Tom Clegg over 2 years ago
- Status changed from In Progress to Resolved