Re: Looking for feedback on service capability semantics

[Claire Gonyeo]

+Gary, in case he's not on this list

On Tue, Nov 19, 2024 at 5:21 PM Claire Gonyeo <cgo...@google.com> wrote:

Hi everyone,

I've spent this year working on refactors in component manager's routing code, and I'm currently in the middle of reimplementing our handling of service capabilities. During code review with Gary, we realized that my new implementation has subtly different semantics than the prior implementation. I could of course rework things to match the semantics of the old system, but the new semantics seem more flexible in an interesting way, so I'm reaching out to DF to ask if you all have any opinions on what is needed, preferred, or disliked here.

The key difference here comes down to whether or not we'd like consumers of filtered or renamed services to see live updates in the set of available instances from service providers.

When a consumer of a service capability that uses renames/filters tries to open a service capability the prior (and current) implementation will:

• Find all service providing components.
• Wait for each providing component to publish the service directory (e.g. `/svc/fuchsia.example.EchoService`) in its outgoing directory.
• Read the set of instances from each service directory.
• Create a new directory and populate it based on the entries in each service directory, following the given filter/rename rules.
• Return the created directory to the service consumer.

Whereas in my new implementation the flow is slightly different. My unmerged changes will:

• Find all service providing components.
• Wait for each providing component to publish the service directory in its outgoing directory.
• Create a dictionary.
• Set up a watcher on each service directory that will update that dictionary appropriately (following the filter/rename rules) whenever a service instance is added or removed.
• Wait for each watcher to reach idle.
• Return the dictionary to the service consumer (who may consume the dictionary by converting it into a `fuchsia.io.Directory`).

(note that the above steps have been simplified for brevity)

In short: the new implementation means that components that consume filtered/renamed services will see the service directory be updated if the instances available to it change.

As an example, imagine we have a service capability that's exposed from component `A` and consumed by component `B`. The service is made available to `B` by an offer with service instance renames from `1` to `2` and `3` to `4`. If `B` opens the service using `fuchsia_component::client::open_service`, and `A` publishes `1` when it starts, then `B` will immediately a service instance named `2`. If `A` at some point in the future then publishes service instance `3`, `B` will then see both service instances `2` and `4` in the same directory channel that it originally opened. If `A` then stops running, `B` will see the same directory channel has no entries. Before my changes, `B` would need to re-open the service (thereby establishing a new `fuchsia.io.Directory` channel) to observe the new instance `4`, and would not be able to observe `A` stopping (as re-opening the service would cause `A` to start again).

I personally like the new semantics, as it more accurately tracks the state of the world and perhaps opens some novel use cases around relying on using a VFS watcher to observe filtered or renamed service instances coming and going, and the live-updating semantics better match the semantics for anonymized service aggregates, but services have a lot of subtle behavior and we want to be very careful to not accidentally change semantics that DF relies on or are planning on relying on.

Sorry for the lengthy email! Does anyone on DF have an opinion or interesting insight on this?

Thanks,

Claire

[Claire Gonyeo]

[Suraj Malhotra]

I don't think this will change anything meaningful for drivers. Parent drivers are required to publish the instances they share with children before the children are created. Therefore by the time the child driver binds and connects the instance should already be present. Additionally we don't actually open the intermediary service directories in drivers because we always know the instance names and assume they are already present and ready to use. We open them with absolute paths instead. If a service instance goes away, that is either a big in the parent driver or it crashed in which case the child driver observing it is of no consequence as that driver will promptly be stopped and unloaded in the near future anyway.

--
All posts must follow the Fuchsia Code of Conduct https://fuchsia.dev/fuchsia-src/CODE_OF_CONDUCT or may be removed.
---
You received this message because you are subscribed to the Google Groups "drivers-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to drivers-dev...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/drivers-dev/CAA338J1EUfeAyRYiaovajgH%3DKN3nakwOk5sGH3D-8wLULkPNUQ%40mail.gmail.com.

[Claire Gonyeo]

Ok, that's great to hear! I'll confer with Gary on the next steps for my change. Thanks for taking the time to look at this.

[Gary Bressler]

> Before my changes, `B` would need to re-open the service (thereby establishing a new `fuchsia.io.Directory` channel) to observe the new instance `4`, and would not be able to observe `A` stopping (as re-opening the service would cause `A` to start again).

I thought that in the original version, `B` would always see `4` if it's in the rename map. If `B` opens `4` that leads to one of the following outcomes: 1) blocks if `A` hasn't served its outgoing dir yet, 2) forwards to `A.2` if `A.2` exists, 3) fails to open if `A.2` does not exist. Is that correct?

> I don't think this will change anything meaningful for drivers. Parent drivers are required to publish the instances they share with children before the children are created.

In the past we've discussed having non-driver components that provide services. For example, we now have the builtin 'service broker' component. Is that still an important use case to consider? In that scenario it's more likely that the provider isn't running yet at the point the client opens the service.

[Suraj Malhotra]

On Thu, Nov 21, 2024, 7:57 AM Claire Gonyeo <cgo...@google.com> wrote:

> If `B` opens `4` that leads to one of the following outcomes: 1) blocks if `A` hasn't served its outgoing dir yet, 2) forwards to `A.2` if `A.2` exists, 3) fails to open if `A.2` does not exist. Is that correct?

Ah, you're right. After my change `B` opening `4` leads to the same outcomes, but where the failure occurs in outcome (3) has changed. `B`'s open request to `/svc/fuchsia.examples.Echo/2` would be rejected by component manager as that directory entry doesn't exist, whereas before my change the open request would be rejected by `A` for the same reason.

I think this change is fine.

On Wed, Nov 20, 2024 at 6:43 PM Gary Bressler <g...@google.com> wrote:

> Before my changes, `B` would need to re-open the service (thereby establishing a new `fuchsia.io.Directory` channel) to observe the new instance `4`, and would not be able to observe `A` stopping (as re-opening the service would cause `A` to start again).

I thought that in the original version, `B` would always see `4` if it's in the rename map. If `B` opens `4` that leads to one of the following outcomes: 1) blocks if `A` hasn't served its outgoing dir yet, 2) forwards to `A.2` if `A.2` exists, 3) fails to open if `A.2` does not exist. Is that correct?

> I don't think this will change anything meaningful for drivers. Parent drivers are required to publish the instances they share with children before the children are created.

In the past we've discussed having non-driver components that provide services. For example, we now have the builtin 'service broker' component. Is that still an important use case to consider? In that scenario it's more likely that the provider isn't running yet at the point the client opens the service.

Still important, just not driver specific. I think this is an improvement for the non-driver user. I would expect the new behavior is what folks would have assumed anyways. If the provider doesn't guarantee the service instance is there when it publishes its outgoing directory then the user needs to use a watcher. 

[Claire Gonyeo]

> If `B` opens `4` that leads to one of the following outcomes: 1) blocks if `A` hasn't served its outgoing dir yet, 2) forwards to `A.2` if `A.2` exists, 3) fails to open if `A.2` does not exist. Is that correct?

Ah, you're right. After my change `B` opening `4` leads to the same outcomes, but where the failure occurs in outcome (3) has changed. `B`'s open request to `/svc/fuchsia.examples.Echo/2` would be rejected by component manager as that directory entry doesn't exist, whereas before my change the open request would be rejected by `A` for the same reason.

On Wed, Nov 20, 2024 at 6:43 PM Gary Bressler <g...@google.com> wrote: