feat(cost-model): implement compute_operation_cost
#44
Conversation
|
@xx01cyx It would be awesome if you can double check the important logic, like |
| if store_output_statistic { | ||
| let res = self | ||
| .storage_manager | ||
| .store_cost(context.expr_id, None, Some(output_statistic.clone()), None) | ||
| .await; | ||
| if res.is_err() { | ||
| eprintln!("Failed to store output statistic"); | ||
| } | ||
| }; | ||
| Ok(output_statistic) |
There was a problem hiding this comment.
This might cause problem if some stats are missed from the storage. We should in the future consider having sth like a retry queue to store failed database requests. But I'm good with this for now. Maybe we can add a comment here.
There was a problem hiding this comment.
Sure, I think the retry should be handled by the storage manager, and I will add a comment there. I don't think it is a big deal, since maybe we can recalculate it again?
|
Also, the logic for invalidating a cost seems to be missing here. I did not comment on the code because I'm not sure what is meant by "valid". The logic should be added after #44 (comment) is resolved. |
I think the logic of invalidating a cost is handled by update_stats? |
Oh yeah you're righg |
compute_operation_cost