dfinity / interface-spec

IC Interface Specification
https://khsfq-wqaaa-aaaak-qckvq-cai.icp0.io/docs
37 stars 20 forks source link

[FINAL] feat: Best-effort responses #268

Closed oggy-dfin closed 2 weeks ago

oggy-dfin commented 10 months ago

To ensure scalability, we want to allow canister authors to use best-effort responses to inter-canister calls, which time out after a period of time.

Details are in IC-1674 and the forum post:

https://forum.dfinity.org/t/scalable-messaging-model/26920

oggy-dfin commented 10 months ago

@mraszyk yes, but I would like to first agree on the signatures and the informal description of the semantics with the stakeholders. Hence the draft status :)

dsarlis commented 6 months ago

@oggy-dfin Should we rephrase the title of this PR to something more descriptive? Like "feat: Introduce best-effort response messages" (I guess the timeout on responses is the implementation detail of how you achieve best-effort responses)?

oggy-dfin commented 6 months ago

@oggy-dfin Should we rephrase the title of this PR to something more descriptive?

Good point, changed now.

lwshang commented 6 months ago

Can we consider renaming call_with_best_effort_response: (timeout_seconds : i32) -> () to something like call_timeout_set : (seconds : i32) -> ().

IMO, this will be more consistent with other call_* API's naming convention:

  1. call_ prefix indicates that this is for making inter-canister calls.
  2. a noun specifies which perspective of the call is being set/modified, e.g. cycles in call_cycles_add.
  3. a verb indicates what action is done to the none. e.g. add in call_cycles_add.

I believe "Best-Effort Response" is a suitable name for an abstract feature. However, for the specific API, I think call_timeout_set would be a clearer and more descriptive name.

oggy-dfin commented 6 months ago

Can we consider renaming call_with_best_effort_response: (timeout_seconds : i32) -> () to something like call_timeout_set : (seconds : i32) -> ().

In the first draft I actually named the call call_set_timeout, eventually changing to the current name:

https://github.com/dfinity/interface-spec/pull/268/commits/ff8d96cb42dd90c1e6f6edd2f435f4794be13659

This was to make it clear already from the name that the system may now throw away responses pretty much at will, completely disregarding the timeout in some cases (in particular, high load). I think that still stands to reason.

mraszyk commented 1 month ago

@oggy-dfin Could you please also resolve the conflicts?

mraszyk commented 2 weeks ago

Superseded by https://github.com/dfinity/portal/pull/3764