-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: wrap-up migration tooling for Azure.Table.Storage -> Azure.CosmosDb.Storage scenario #8
base: proto/state-migration
Are you sure you want to change the base?
Conversation
src/Azure/Orleans.Persistence.AzureStorage.Migration/AzureStorageOfflineMigrator.cs
Outdated
Show resolved
Hide resolved
test/Extensions/Tester.AzureUtils.Migration/Tester.AzureUtils.Migration.csproj
Outdated
Show resolved
Hide resolved
test/Extensions/Tester.AzureUtils.Migration/OrleansTestSecrets.json
Outdated
Show resolved
Hide resolved
...e/Orleans.Persistence.AzureStorage.Migration/Reminders/MigrationAzureTableReminderStorage.cs
Outdated
Show resolved
Hide resolved
Reminder
entity// options.ContainerName = $"destination{RandomIdentifier}"; | ||
options.ContainerName = $"destinationtest"; | ||
options.DatabaseName = "Orleans"; | ||
options.ConfigureCosmosClient(TestDefaultConfiguration.CosmosConnectionString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont know if we can put an emulator here? I was testing against my own cosmos instance in Azure
@@ -314,18 +315,21 @@ private object ConvertFromStorageFormat(byte[] contents, Type stateType) | |||
|
|||
public async IAsyncEnumerable<StorageEntry> GetAll([EnumeratorCancellation] CancellationToken cancellationToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would have been nice to skip alredy migrated entries, but I guess it's ok since we only get metadata? But we still create a new entry everytime.
EDIT: ah no, you read the entire entry everytime. I wonder if we should so a lazy read, done only if we explicitely read the entry to migrate it. That might need some new types though... Did you tested it with a large collection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you read the entire entry everytime
that is correct. I did not test with the large collection (it is basically like the implementation existed with AzureBlobStorage - I just added loading the metadata). So you want to do something like that?
var metadata = client.LoadMetadata(entry);
if (metadata is null || metadata.IsNotMigrated())
{
var blob = client.LoadBlob();
yield return blob;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename the method to GetNonMigrated
entires or smth like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename the method to GetNonMigrated
entires or smth like that + only fetch the whole entry if it is not migrated (as proposed in previous comment)
return cosmosGrainStorage.WriteStateAsync(grainTypeData, stateName, grainReference, grainState); | ||
} | ||
|
||
private GrainStateTypeInfo GetGrainStateTypeInfo(GrainReference grainReference, IGrainState grainState) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move to interface to share the same cache as done in CosmosGrainStorage
return cosmosGrainStorage.WriteStateAsync(grainTypeData, stateName, grainReference, grainState); | ||
} | ||
|
||
private GrainStateTypeInfo GetGrainStateTypeInfo(GrainReference grainReference, IGrainState grainState) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move to interface to share the same cache as done in CosmosGrainStorage
@@ -314,18 +315,21 @@ private object ConvertFromStorageFormat(byte[] contents, Type stateType) | |||
|
|||
public async IAsyncEnumerable<StorageEntry> GetAll([EnumeratorCancellation] CancellationToken cancellationToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename the method to GetNonMigrated
entires or smth like that
Current PR attempts to finish existing draft of migration tooling to support a specific scenario of migration from Azure.Table.Storage to Azure.CosmosDb.Storage (other scenarios such as blob migration is supported as well).
Migration consists of:
Migration tooling provides API to control the migration in different phases:
DataMigrator
API to invoke a force migration of data from "old storage" to "new storage" (even if grain is not touched from the runtime)In this PR i have moved an existing "forked" Orleans.Persistence.Cosmos implementation.
For each of the
Orleans.Persistence.XXX
project exists aOrleans.Persistence.XXX.Migration
project, which contains new types, which allow writing data in new format, and SiloExtensionHelpers to register those types in DI.Tests are covering the cases of migration from one to another storage.