OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.38k stars 2.38k forks source link

Background tasks should wait for migrations to finish #8697

Open scleaver opened 3 years ago

scleaver commented 3 years ago

Describe the bug

I have two background tasks that run every minute (kind of like the way the PublishLater module background task). I recently had to push some changes to Indexes in a migration step to a multi-tenanted solution. Theses changes required updating all content items that used the index so it wasn't a very quick migration.

Before the migration were completed for all tenants the background tasks fired and caused errors because the migration tasks hadn't completed. At least this is what appeared to happen.

Is there any way to solve this... like somehow halt background tasks while migrations are completing?

Skrypt commented 3 years ago

So, kind of like what I said to @jtkech the other day. These background tasks should not be stopped if their intent is to mutate data. For example ; a publish later task that should publish a content item at X datetime. The timing is also important.

So, if you need to publish a content item while it is beeing mutated, your Workflow or Background task needs to support both data structure "old and new" when you are doing the migration. And then, you can drop the old data structure conditions when it's migrated.

Else, disabling these background tasks will get you the same outcome in the end. Which is not what is needed.

An other option is alway to execute a SQL Query directly and to stop the website while doing so. Which will get you the same outcome too. You might miss the publishing of a content item because the website was down while it should have published the content item...

Here, we would need to know when the app has stoped the last time and which background tasks did not execute since then. Then execute these background tasks. Here, same thing if we could "pause" the background tasks for some time.

This is an interesting suggestion and I think it is worth investigating solutions.

I think the method you want to look into is the CanRun() one in the BackgrounTaskScheduler. Extract the logic of that one and see what can be done.

scleaver commented 3 years ago

@Skrypt - yeah that makes sense. It looks like the Background Tasks are only caught up in the issue.

My issue was actually that I did two index modifications via migrations in one release which required the content to be updated. Both indexes were used by the same content. One migrated then ran the content update to populate the new index which then of course caused an error because the other migration hadn't run yet and it's index had missing columns.

The way around it was to disable the content update code. Deploy and allow the migrations to run. Enable the content update in another step and deploy again.

Note that the background task was the first to log an error but as soon as I disabled background tasks then I noticed the other error.

So what I think is needed is a solution for both:

  1. A way to pause background tasks before migrations is finished - note in my opinion if a background task runs once a migrations is done and it was delayed by 2 mins then this is not such an issue. I guess others might need the tasks to execute right on time.
  2. A way to process content updates after all migrations are done.
jtkech commented 3 years ago

Some infos

Normally the background service doesn't call a background task on a tenant that is not yet activated, in fact we check if its pipeline has been built which happen after the activation, and migrations happen while the tenant is activating.

How do you trigger the migrations, by restarting the app, reloading a tenant through the Admin? Hmm, in the last case maybe possible that the bg task was already started in a scope of the previous shell. If so in your code you could check if the current shell is released (not disposed because still in use), this to stop the work on a scope of a released shell.

You can enable the BackgroundTask feature, it allows through the admin to pause / restart a background task.