-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Is there not a way to use a custom serializer with Ado storage? #6257
Comments
@jamcar23 You are not missing anything, but read on. This structure is from 1.n era when there was no dependency injection. While it would have been cumbersome in 1.n world to do what you are thinking (e.g. using reflection) the facilities were added to make it possible and to have some tests to do it. It would make sense to expose a way to add a custom (de)serialize through options. Worth to point out, the idea is to be able to pick a (de)serializer on per instance basis which also leads to the possibility to do data transformations on per instance basis. Two use cases are switching between (de)serialization formats and refactoring (namespaces, class names). I'm fairly sure in the future there would need to be a more holistic approach that allows one to flow data through the system, but for your purpose I think the shortest path to do this is to refactor so that the (de)serialization can be passed explicitly from the configuration. |
If I was to refactor this would the orleans team be interested in a pull request for this? Or, do they have other things in pipeline and wouldn't be interested in it? If I was to submit a PR, is there anything else I would need to know (such as having to refactor the options for all storage providers, etc.)? |
I had a similar need as I dont want the state be serialized in JSON or in single column but have more control on which SQL table I write to and read from. So that I have more schematized table format for read and search paths in system. Also giving ability to change ConnectionString for provider based on grain reference is an easy way to point to partitioned data across multiple databases. I am ending up creating my own provider :( which I would like to avoid and just extend on existing ADONet providers. |
@jamcar23 I am not to decide on behalf of the team, maybe @sergeybykov can tell. I am not entirely clear what is involved in refactoring in a wholle user provided (de)serialization library. I think you need to start from https://github.com/dotnet/orleans/blob/master/src/AdoNet/Orleans.Persistence.AdoNet/Options/AdoNetGrainStorageOptions.cs and have it to use either the current interfaces or lambda. Either way, I personally just would like to retain the ability to pick and choose with any parameters (including the contents) as currently so as to keep the ability to do transformations and switching formats with deplyoments. I do not think other providers need to be synchronized with these changes. The tests need to be modified a bit, but doesn't look like a big change if one needs to use constructor injection or along those lines. @sammym1982 You do not need to use storage persistence with Orleans, you can use storage however you like with grains. If you think about it, the Orleans persistence mechanism is a generalization so that you can persist something as blobs using the grains as identifiers. This implementation is one that tries to be fairly maintenance free and performant while still giving one the ability to consider the physical and logical storage aspects. That being noted, you can naturally change the storage layout and queries as one mechanism. Orleans cares just that you have the names and types correctly (including return value). The rest is that it uses the ID of the query, fills in the parameters and sends the computation along with data to the database. In https://github.com/dotnet/orleans/tree/master/Samples/OneBoxDeployment you can see there are some slight modifications so that persistence is on its own filegroup and schemas are used. This shows one example of example security and availability (and disaster recovery) aspects. One could also create multiple persistence tables to different filegroups and modify the I'm on a business trip this week, sorry for the sluggish answer. |
Adding a way to configure the serializer for this provider (and eventually others) would be a welcome contribution |
I already talked about an idea here: #5738 (comment) The idea would be to provide a class like protected abstract Task ReadStateAsync(string grainType, GrainReference grainReference, IGrainState grainState, DbConnection connection);
protected abstract Task WriteStateAsync(string grainType, GrainReference grainReference, IGrainState grainState, DbConnection connection);
protected abstract Task ClearStateAsync(string grainType, GrainReference grainReference, IGrainState grainState, DbConnection connection); It would probably need a few more base classes like an Like that, it will be easier to support more DB types or to customize how the data are stored. Don't know if it makes sense? |
It's unclear to me what would you do with the raw I imagined something more related to serialization to/from bytes than to/from a database. |
Ok, maybe I'm a bit off topic... my suggestion is more for the issue @sammym1982 has
For your questions
It allows the user to write/read the data where he wants and how he wants.
I don't really know the effort it takes to create a robust AdoNet IGrainStorage from scratch. I think it would be interesting to have a base infrastructure that can be easily extended to support more DB types, while doing all the plumbing tasks. The problem I see with the existing ADO provider is that it is not very flexible and it only works with DB with stored procedures support. |
That's fair, @srollinet - I think your approach is doable. It may be too much control for many users who just want to plug in some custom serializer, but it might have value to some. A proposal with code could help to illustrate the value |
@srollinet The SQL Server provider does not use stored procedures, for instance: https://github.com/dotnet/orleans/blob/master/src/AdoNet/Orleans.Persistence.AdoNet/SQLServer-Persistence.sql. The problem one may have is that is it possible to use variables without stored procedures (and how to efficiently construct a needed query without a temporary variable). There could use cases that should have a persistence API in Orleans, or ADO.NET specifically, that allows one to run arbitrary queries to the database? If so, should it be an application concern and one use EF, Dapper or others? If not EF, Dapper or something else, would other construct than protected inheritance work (that inheritance creates dependencies). |
We need to extend the functionality which @benjaminpetit implemented in #7416 for Azure to the other providers, including ADO.NET (this issue) and, in doing so, remove the current custom serializer support from the ADO.NET providers |
@ReubenBond I can help testing if that is needed. I remember based to #7416 is From that it looks to me the change is to remove the "picker" functionality that decides which (de)serializer to use inside AdoNetProvider. After refactoring it should come via constructor injection in options. This requires changes to tests and removing those tests that change (de)serializer dynamically. This is additional: I would be interested to know if there is a way:
It might be doable by configuring a custom |
We've moved this issue to the Backlog. This means that it is not going to be worked on for the coming release. We review items in the backlog at the end of each milestone/release and depending on the team's priority we may reconsider this issue for the following milestone. |
Looking at the docs it seems like one would only need to implement an
IStorageSerializer
and anIStorageDeserializer
but then when looking at the line below inAdoNetGrainStorage
:orleans/src/AdoNet/Orleans.Persistence.AdoNet/Storage/Provider/AdoNetGrainStorage.cs
Line 401 in d0203d2
there doesn't seem to be a way to pass a custom serializer into the grain storage. Am I missing something or is there really no way?
The text was updated successfully, but these errors were encountered: