Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Expose ability to set IContainerRuntimeOptions to AzureClient #12275

Closed
DLehenbauer opened this issue Oct 5, 2022 · 12 comments
Closed
Assignees
Labels
area: azure-client Work related to the azure-client pkg. This is generally about exposing new functionality. status: stale triage

Comments

@DLehenbauer
Copy link
Contributor

DLehenbauer commented Oct 5, 2022

Work Item

We commonly use IContainerRuntimeOptions to stage changes or test new and experimental features. For example, we are currently using IContainerRuntimeOptions to enable/configure op compression. Because IContainerRuntimeOptions is not currently exposed by AzureClient, users using the AzureClient lack a convenient way to help co-develop or test/measure the impact of runtime changes. (Related: #11572, #12283)

Approach
A mechanism that allows users of AzureClient to provide a set of desired IContainerRuntimeOptions.

Open questions
@ChumpChief points out that the valid set of IContainerRuntimeOptions are specific to a particular container implementation. It's worth considering if there are disadvantages to publicly binding the AzureClient version to a specific container implementation and version, or if there is a way to avoid this.

We also may want to consider if we want to expose all runtime options, or if there are options we would like to hide from the AzureClient. Anecdotally, past users have discovered and enabled vestiges of old prototype chunking code.) Possibly IContainerRuntimeOptions should have "public" and "private" sections.

Acceptance criteria
The goal is to enable AzureClient users to co-develop new or experimental features or help test/measure the impact of planned runtime changes.

@ghost ghost added the triage label Oct 5, 2022
@DLehenbauer DLehenbauer added the area: azure-client Work related to the azure-client pkg. This is generally about exposing new functionality. label Oct 5, 2022
@ChumpChief
Copy link
Contributor

I thought about this a bit more last night as well. I think there's a bit of a scenario mismatch here too. A big part of the initial motivation for AzureClient/FluidContainer was to permit customers to use Fluid data without having to know about, think about, or write a container runtime. For customers of AzureClient/FluidContainer who later decided they did want to influence the container runtime, the solution would be to peel back that layer and use the underlying API surface (Loader/Container/IRuntimeFactory etc.). By default I think this request would fall under that category, though of course we can consider options here.

I do think we need to think critically about the scoping here though if we do relax this initial approach - I think AzureClient/FluidContainer have a clear purpose today (to scope and simplify our API surface with the acknowledged tradeoff of advanced functionality and customization), but if they eventually just become a passthrough for everything then I think that clear purpose becomes less clear.

@milanro
Copy link
Contributor

milanro commented Oct 17, 2022

For customers of AzureClient/FluidContainer who later decided they did want to influence the container runtime, the solution would be to peel back that layer and use the underlying API surface (Loader/Container/IRuntimeFactory etc.). By default I think this request would fall under that category, though of course we can consider options here.

Hello @ChumpChief , I understand the reasoning for having AzureClient with zero configuration to enable the customers with no insight to get quickly on. On the other hand, with this approach, any very small deviation from the default setup requires highly increased complexity where actually customer still could deal with a simple API of AzureClient and IContainerRuntimeOptions object. Couldn't we have AzureClient working as today with the very simple APi but still be able to force our IContainerRuntimeOptions via further set method optionally executed after the construction of the AzureClient?

@ChumpChief
Copy link
Contributor

Another thought - AzureClient specifically isn't a great fit for these options because it's analogous to the Loader (which also doesn't take an IContainerRuntimeOptions as it would be a layer violation).

If we do relax our past scoping decisions, we'd most likely want to do so by exposing the option to plug a runtime factory (i.e. a specially configured DOProviderContainerRuntimeFactory) into the AzureClient rather than spaghetti the config through AzureClient to the runtime factory. This would align better with the underlying architecture, avoid another layer violation, and set up for the inevitable next request for alternate runtime factory implementations. My concerns above about purpose and scoping of this package still stand though.

@milanro
Copy link
Contributor

milanro commented Oct 19, 2022

@ChumpChief Indeed the constructor of DOProviderContainerRuntimeFactory right now hardcodes IContainerRuntimeOptions at the fluid-static/src/rootDataObject.ts. If I understand well, that constructor could optionally accept it as a parameter and that factory could then be given to the createLoader method at the AzureClient (and supplied by client at the AzureClient API). Of course, the client will have to deal with both DOProviderContainerRuntimeFactory and IContainerRuntimeOptions but it is still simple approach. Is this what you mentioned?

@ChumpChief
Copy link
Contributor

Yes, something like that - might provide it to AzureClient in some slightly different way (e.g. via a code loader perhaps) but along those lines.

@DLehenbauer
Copy link
Contributor Author

DLehenbauer commented Oct 19, 2022

FYI - This is AB#2166 in our backlog. I spoke w/Skyler abt. this yesterday and learned I dropped the ball by not looping in Pranshu. Pranshu will reach out soon.

@skylerjokiel
Copy link
Contributor

Ejecting to a Loader/ContainerRuntime based development model is not something we support for Azure Fluid Relay. While this may be a path we explore in the future it's not something developers can succeed in doing currently (no docs/examples/anything). AzureClient is designed to be the way developers use Azure Fluid Relay.

Fluid will continue to develop new features that are internal to the inner layers (container/runtime/driver/ect.) It seems very reasonable that we will need a way to expose new functionality to Azure developers in an opt-in manor. So I would reframe the problem from saying, "we expose the ability to set IContainerRuntimeOptions at the AzureClient layer," to, "we should provide feature flags that allow us to expose any experimental feature functionality to azure developers." In this case we want to expose the ability to enable "op compression" as we develop it.

@dstanesc
Copy link
Contributor

Elevating the discussion from IContainerRuntimeOptions to feature flags meets our goal. The only important corollary to emphasize is the relevance of the sibling Request - 12283, the ability to configure DDSes. Ideally the upcoming design should address both. For the latter, the distinction comes from:

  1. Allowing the Apps not only to enable/disable but also to adjust particular DDS features to the domain (eg. aligning summary chunk granularity to the use case)
  2. Configuration to be provided at DDS instantiation time

The large data support roadmap fruition depends on both.

@ChumpChief
Copy link
Contributor

If the feature flags are IContainerRuntimeOptions-specific then there's no material difference -- the same arguments would apply for/against passing them through the AzureClient vs. a configurable runtime factory.

@microsoft-github-policy-service
Copy link
Contributor

This PR has been automatically marked as stale because it has had no activity for 60 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

@milanro
Copy link
Contributor

milanro commented Feb 2, 2023

We should probably keep this opened.

@microsoft-github-policy-service
Copy link
Contributor

This issue has been automatically marked as stale because it has had no activity for 180 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: azure-client Work related to the azure-client pkg. This is generally about exposing new functionality. status: stale triage
Projects
None yet
Development

No branches or pull requests

5 participants