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

Implement rehydration for LROs #2158

Closed
m-nash opened this issue Apr 12, 2022 · 16 comments · Fixed by Azure/azure-sdk-for-net#42740
Closed

Implement rehydration for LROs #2158

m-nash opened this issue Apr 12, 2022 · 16 comments · Fixed by Azure/azure-sdk-for-net#42740
Assignees
Labels
DPG/RLC Backlog DPG Mgmt This issue is related to a management-plane library. WS: Code Generation

Comments

@m-nash
Copy link
Member

m-nash commented Apr 12, 2022

No description provided.

@ArthurMa1978
Copy link
Member

Azure/azure-sdk-for-net#27409 customer asked for this

@annelo-msft
Copy link
Member

annelo-msft commented May 11, 2022

@m-nash, do you plan to address this for data plane as well, or just management plane as part of this issue? Data plane will need it for the v1.1 of the DPG generator.

We will need to provide an implementation for Id where we currently throw a NotSupportedException: https://github.com/Azure/autorest.csharp/blob/feature/v3/src/assets/Generator.Shared/LowLevelFuncOperation.cs#L29

In order for this to work for all LRO types (if we want that), we would need to save the following in the Id string:

  • Azure-AsyncOperation header (value or absence)
  • Location headers (value or absence)
  • Operation-Location (value or absence)
  • Initial body (there are situations where it may be the final result)
  • Initial URL (there are situations where we need a final call on the initial Url)

If we can confirm that we know what LRO pattern is implemented for a given service, we can store just the information needed per service.

Making this work across languages is a nice to have, but not a requirement, per @mikekistler.
@lmazuel, @KrzysztofCwalina, @AlexanderSher, @lirenhe FYI.

@annelo-msft
Copy link
Member

This should be preceded by #2231, to make the process more efficient.

@archerzz
Copy link
Member

A similar customer issue:Azure/azure-sdk-for-net#13625

@archerzz
Copy link
Member

@m-nash @annelo-msft
A user (@madd0) of mgmt track2 SDK is asking for the same feature. Previously he can use UseClientContext to resume the polling, but that method was removed in later version. They depend on the rehydration feature to implement their work.

@madd0
Copy link

madd0 commented Jun 15, 2022

As explained here, in our scenario we obtain an Operation instance in one process, but poll its progress in another. To do so, what we're doing is persisting the URL provided in the HTTP headers in storage.

Since we needed to update the version of the SDK, we are now using the HttpPipelineBuilder to execute the request in a context similar to the one that would've been used by the ArmClient that provided the initial operation object.

If a dehydrated version of the operation can be persisted to storage and then rehydrated to provide status updates, that would definitely suit our needs, without having to depend on low-level APIs which are susceptible to change.

@adstep
Copy link

adstep commented Oct 22, 2022

Our team is also looking for extended support for tracking long running operations. We run a distributed service and the instance that would be polling for the LRO might not be the one that started it.

@droyad
Copy link

droyad commented Dec 20, 2022

This would be helpful for us too. At the moment we can't sleep tasks that wait for infrastructure to be created, so they don't survive application restarts.

@m-nash
Copy link
Member Author

m-nash commented Jan 3, 2023

@annelo-msft definitely looking to implement a feature that can be used for both data plane and mgmt plane as well as rehydrating across languages where possible. I know in medium to large sized organizations different parts of a microservice system can be written in different languages so having the ability to start in one language, store the id then restart in another language is ideal.

@m-nash
Copy link
Member Author

m-nash commented Jan 27, 2023

@fengzhou-msft After looking at each of the prototypes I think we are going to bring in the general public serialization issue as the first step here.

We won't use any of the new static interfaces introduced in dotnet 7 but instead have an ISerializable (name TBD) interface with 2 instance methods. This will require us to have parameterless constructors which we should keep internal and will access via Activator. Once constructed we can try cast the object into the interface and call deserialize.

We also want switch from static Rehydrate method to a constructor. Originally we avoided constructor since in general we should never throw from a constructor, but we feel the usability outweighs the edge case of when a throw could occur.

For a strawman on the interface please use

public interface ISerializable
{
    bool TrySerialize(Span<byte> buffer, out int bytesWritten, StandardFormat format = default);
    bool TryDeserialize(ReadOnlySpan<byte> data, out int bytesConsumed, StandardFormat format = default);
}

For the interoperability with the current Operation infrastructure I think we use the internal GenericOperationSource class which will handle constructing T from Activator and try casting into the ISerializable and this way we don't have to change what all of that internal code is expecting.

The only tricky part here is when the T is a ResourceClient. We don't want to add ISerializable to ResourceClients as the entire object is not serializable just the data. The ISerializable should go on the Data and we will need a way to get the Data from the Resource. Unfortunately we can't simply use name manipulation as sometimes the data is shared between ResourceClient types so it N : 1. Let me know what you think could be viable options here.

I think we can split the work here I will start the PR to introduce this ISerializable interface and work through all the model versioning issues, in the meantime we can consolidate the rehydration prototype PR into one and you can just add this interface into Azure.Core in that prototype to test everything else out E2E.

For now the implementation of that interface should be hidden so new methods don't show up in the public api surface for every model and it should wrap the Write and Deserialize[Model] methods that already exist as a first draft.

We also want to drop any Response reconstruction in the rehydration. This basically means we can simplify the format of the ID to be just the set of primitive values that are needed by the operation internals. This does mean that if a customer rehydrates and passes Operation to a common helper method they will need to check if GetRawResponse is null before accessing it. Any current HLC hand written rehydration scenarios already have this same behavior so this is not a breaking change in behavior. This also shrinks the size and complexity of the id itself. Please add here the final id format so we can discuss that point individually.

cc @tg-msft @KrzysztofCwalina

@KrzysztofCwalina
Copy link
Member

Some additional context/comments:

  1. When I suggested the interface above, I intended the format to represent a version too. Something like new StandardFormat('J', 5) meaning serialize JSON version 5. But I am not sure this is the best representation. In particular, the integer in StandardFormat is called "precision", not "version" and its range is from 0-255.
  2. We probably cannot call this interface ISerializable
  3. Models should implement the interface methods explicitly.

@archerzz
Copy link
Member

Another team is asking for rehydration: Azure/azure-sdk-for-net#29180 (comment)

@ArthurMa1978
Copy link
Member

ArthurMa1978 commented Jan 11, 2024

Release beta by end of 1/12, GA pending on archboard review on 1/18

@RoLfBOT
Copy link

RoLfBOT commented Apr 13, 2024

@ArthurMa1978 any update on the rehydration part?

@ArthurMa1978
Copy link
Member

@RoLfBOT it's delayed, and hopefully can release by this month, April.

@PepijnGramberg
Copy link

Any idea when this will be implemented in Azure.ResourceManagement.Sql? because I'm getting not implemented messages on this still #31549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment