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

Refactor Orleans.Streaming into separate packages #6942

Merged
merged 8 commits into from Feb 9, 2021

Conversation

ReubenBond
Copy link
Member

Ideally, the core of Orleans only contains the basic functionality. This is a first-draft at moving the core streaming functionality out into separate packages: Orleans.Streaming & Orleans.Streaming.Abstractions.

We could aim for 3+ packages or 1 package, instead, depending on how we want to divide things.

@ReubenBond ReubenBond added this to the 4.0.0 milestone Feb 8, 2021
@ReubenBond ReubenBond marked this pull request as draft February 8, 2021 00:56
@ReubenBond ReubenBond force-pushed the streaming/move/1 branch 2 times, most recently from 139417f to 1fa8462 Compare February 8, 2021 21:28
@ReubenBond ReubenBond marked this pull request as ready for review February 9, 2021 00:21
return streamDirectory;
}

public async Task Reset(bool cleanup = true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this method be called concurrently?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only called during shutdown, so it shouldn't be

@benjaminpetit
Copy link
Member

I think for now two packages is better than what we have now.

We can see later if we want to move MemoryStream and SimpleMessageStream in different package later on, in smaller PR

@benjaminpetit benjaminpetit merged commit d59798a into dotnet:master Feb 9, 2021
@ReubenBond ReubenBond deleted the streaming/move/1 branch February 10, 2021 00:42
@pentp
Copy link
Contributor

pentp commented Feb 14, 2021

From a user's perspective it's better to have fewer separate packages if they are just for Orleans internal code separation. Even then, if this separation is really needed, it would be better to achieve by packaging multiple dll-s in the same nuget package.

However, the reverse is true if the packages pull in additional external dependencies.
One particular pain point for me was that Orleans.Persistence.AzureStorage depends on both Microsoft.Azure.Cosmos.Table and Azure.Storage.Blobs, but you'd typically actually only use one of these (the latter used to be Microsoft.Azure.Storage.Blob and we avoided any versioning issues by using Azure.Storage.Blobs in our own code).

Ideally, Orleans.[Clustering/GrainDirectory/Persistence/Reminders].AzureStorage should all be merged into one (e.g., Orleans.Azure.Cosmos.Table or something like that) as they all have identical dependencies (and a lot of duplicated code files actually) and blob storage provider split out into its own package (e.g., Orleans.Azure.Storage.Blobs).

Related to this, we should start multi-targeting Orleans.Core.Abstractions package to avoid the additional dependencies on .NET Core and to enable using newer APIs in core abstractions code.
And Orleans.Runtime.Abstractions should be merged into Orleans.Core as it's not really a classic abstractions package because it pulls in everything that Orleans.Core uses already and doesn't contain much code actually.
Orleans.Server and Orleans.Client could be removed entirely.

@ReubenBond
Copy link
Member Author

I agree with what you're saying. In this case, moving these pieces into a separate project is intended to help us iterate on them more quickly and keep the core more agile (less special cases, pay only for what you use in each activation/call). Even if we package them all into Orleans.Sdk/Orleans.Server/Orleans.Client or whatever metapackages we end up with (and I think we should aim for one or two packages per end-developer project. CodeGen should be included automatically as a part of the SDK package)

The existing packaging scheme made sense originally, before Cosmos DB took over the SDK for one Azure Storage service but not the other... Maybe now is time to split them.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants