Document resource limiting #510
Conversation
| @@ -404,9 +404,50 @@ impl Builder { | |||
|
|
|||
| /// Register a new resource kind. Errors if `label` is already registered, or if number of | |||
| /// registered resources would exceed 8. | |||
| /// | |||
There was a problem hiding this comment.
The thing that worries me about these duplicated docs is keeping them both in sync in the event that things change. I don't think that there is a nice way to share documentation though is there?
I'd be tempted to have one version of the docs link to the other version eg something like
/// Register a new reousrce kind. See [`crate::http_server::Builder::register_resource()`] for example usage.
There was a problem hiding this comment.
I don't think that there is a nice way to share documentation though is there?
I don't know of a way either and it's quite common to see "look over here for details" in rustdocs.
There was a problem hiding this comment.
I don't know of a way either and it's quite common to see "look over here for details" in rustdocs.
Yes, that's what I meant at least.
There was a problem hiding this comment.
I still have the duplicated comments on these, but the bulk of it has been moved to the resource_limiting module, link included, so I reckon this is resolved now?
http-server/src/server.rs
Outdated
| /// # | ||
| /// #[rpc(server)] | ||
| /// pub trait Rpc { | ||
| /// #[method(name = "my_expensive_method", resources("cpu" = 5, "mem" = 2))] |
There was a problem hiding this comment.
In the case of the macro, what is the default resource usage? IIRC (but I am rather sketchy on this code) you define the default usage when creating the Resource object, so I guess if you use the macro, it will define some default defaults? (I'd assume that the default defaults are to use 0 of a resource, and so if you don't specify resource usage in the macro, the method is assumed to use nothing?)
Either way; perhaps it'd be useful to make this clear?
There was a problem hiding this comment.
Either way; perhaps it'd be useful to make this clear?
I agree. It is not immediately obvious. I think one important piece to understand this is to first grok the way resources are set up: not here, around the trait marked with #[rpc] or at the method definition site; resources – along with their capacity and default cost – are set up on the server when it is constructed.
The reason for this separation is that the server app might want to make the resource set up configurable (through CLI or config file) so that different instances of the server can use different caps/defaults (e.g. fast servers == higher caps). It is also technically tricky (impossible?) to have a resource defined on the rpc modules and then somehow coalesced into a server-wide singleton.
So there's a UX problem here, but one we can't really fix. I think the docs should explain this important piece, as it makes understanding the rest much easier.
http-server/src/server.rs
Outdated
| /// # | ||
| /// #[rpc(server)] | ||
| /// pub trait Rpc { | ||
| /// #[method(name = "my_expensive_method", resources("cpu" = 5, "mem" = 2))] |
There was a problem hiding this comment.
Either way; perhaps it'd be useful to make this clear?
I agree. It is not immediately obvious. I think one important piece to understand this is to first grok the way resources are set up: not here, around the trait marked with #[rpc] or at the method definition site; resources – along with their capacity and default cost – are set up on the server when it is constructed.
The reason for this separation is that the server app might want to make the resource set up configurable (through CLI or config file) so that different instances of the server can use different caps/defaults (e.g. fast servers == higher caps). It is also technically tricky (impossible?) to have a resource defined on the rpc modules and then somehow coalesced into a server-wide singleton.
So there's a UX problem here, but one we can't really fix. I think the docs should explain this important piece, as it makes understanding the rest much easier.
Co-authored-by: David <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
Co-authored-by: David <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
No description provided.