Skip to content

Fix for MySql clustering script - #6097

Closed
oleggolovkovuss wants to merge 1 commit into
dotnet:masterfrom
oleggolovkovuss:patch-1
Closed

oleggolovkovuss wants to merge 1 commit into
dotnet:masterfrom
oleggolovkovuss:patch-1

Conversation

@oleggolovkovuss

Copy link
Copy Markdown
Contributor

The script is failing because the OrleansQuery table is not created before trying to insert something there. I think it was lost during "Split something" refactoring which this fix is copied from. I think all other clustering ADO.Net scripts have the same problem

The script is failing because the `OrleansQuery` table is not created before trying to insert something there. I think it was lost during "Split something" refactoring which this fix is copied from. I think all other clustering ADO.Net scripts have the same problem
@oleggolovkovuss

oleggolovkovuss commented Nov 8, 2019 •

Copy link
Copy Markdown
Contributor Author

Looks like the same problem exists for the persistence and reminders setup scripts as well. Also all of the MySQL scripts aren't working from the C# code (using MySql.Data) because of the DELIMITER command

@sergeybykov

Copy link
Copy Markdown
Contributor

@veikkoeeva Do you approve?

@veikkoeeva

veikkoeeva commented Nov 9, 2019 •

Copy link
Copy Markdown
Contributor

The required part is at https://github.com/dotnet/orleans/tree/master/src/AdoNet/Shared and needs to be deployed first.

The rationale for splitting was that not all tables and queries are needed in all cases so split like this to only those that are needed. Since this is the case, maybe this isn't the right fix but instruct people to deploy the required part first.

Though it may be worth considering if the scripts should be merged into one. This would require a bit changes to testing too. Though if one does this, it's tempting to consider removing the #ifdef constructs, have a dll for the core pieces with the merged script and then separate packages for reminders, membership etc. that has the small pieces of code for that specific functionality.

@oleggolovkovuss

Copy link
Copy Markdown
Contributor Author

@veikkoeeva oh, thanks, I see. I can't find any mention of Shared either at
https://dotnet.github.io/orleans/Documentation/grains/grain_persistence/relational_storage.html
or at
https://dotnet.github.io/orleans/Documentation/clusters_and_clients/configuration_guide/adonet_configuration.html
though so I thought it was lost during refactoring and I had to recreate it myself. I guess a complete setup sample from C# would be helpful

@veikkoeeva

veikkoeeva commented Nov 11, 2019 •

Copy link
Copy Markdown
Contributor

@oleggolovkovuss The docs seem to be perpetually in process to be improved. :) You are right of course that this should be told better.

There is one bigger sample at https://github.com/dotnet/orleans/tree/master/Samples/OneBoxDeployment if you are interested. It uses dacpac and so is not directly applicble to your case. You could though switch the connection strings to point to MySQL and switch the ADO.NET connector library.

Related to the previous single script consideration, if there were a single script and it were available in known location, it could deploy it to database during setup. The tests do this, for instance. It has occurred to me to enhance the ADO.NET configuration surface so that there'd be an extension method like Deploy that would take the database name (and ADO.NET vendor information) and create the database and deploy the scripts there. It likely would be helpful to many people while still maintaining flexibility to run any tool one might use in one's enterprise, dealing with security or whatever other concerns there may be. This wasn't possible before Orleans 2.n and the Core like DI features, but would be now.

@sergeybykov

Copy link
Copy Markdown
Contributor

I submitted #6118 to add information about Shared/Main SQL scripts.

@sergeybykov

Copy link
Copy Markdown
Contributor

Now that the docs were updated, should we close this?

@oleggolovkovuss

Copy link
Copy Markdown
Contributor Author

@sergeybykov Besides a small typo Persiatence I guess so, thanks for the feedback!

@oleggolovkovuss
oleggolovkovuss deleted the patch-1 branch November 15, 2019 08:08
@sergeybykov

Copy link
Copy Markdown
Contributor

Besides a small typo Persiatence I guess so, thanks for the feedback!

Thanks for pointing it out. I submitted #6124 to fix it.

@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.

3 participants