OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

Infinite Indexing tasks and YSOD #4498

Open orchardbot opened 10 years ago

orchardbot commented 10 years ago

MrGenius created: https://orchard.codeplex.com/workitem/20669

Hi,

I recently noticed that at some scenario, update index task gets created continuously.

DefaultProcessingEngine.ExecuteNextTask=>UpdateIndexScheduler.Schedule=>DefaultProcessingEngine.AddTask=>DefaultProcessingEngine.ExecuteNextTask=>UpdateIndexScheduler.Schedule.............................................

My observation has been this.

On DefaultOrchardHost.EndRequest, the system tries to execute all the pending tasks. If the pending task is IIndexNotifierHandler.UpdateIndex, it calls the UpdateIndexScheduler.UpdateIndex method. But this again schedules a new task. So this never ends. Now the system is too busy adding, executing and removing task entries and the user is waiting for the page to get loaded. At the end of this task war, the user sees a yellow screen of death.

orchardbot commented 10 years ago

@Piedone commented:

Which Orchard version are you experiencing this?

orchardbot commented 10 years ago

MrGenius commented:

Orchard 1.7.2

orchardbot commented 10 years ago

@Piedone commented:

Would you mind trying with 1.8? There was a related fix that I believe will solve your issue.

orchardbot commented 10 years ago

MrGenius commented:

Sure. Do you have the issue id or changeset so that I can take code only for the affected modules. Migrating entire project may not be an option now.

orchardbot commented 10 years ago

@Piedone commented:

I can't find it now, sorry, but AFAI remember it was done in the Orchard.Indexing module (and probably it was even me who did it?).

orchardbot commented 10 years ago

AimOrchard commented:

Found that changeset yet? This interests us as well.

orchardbot commented 10 years ago

MrGenius commented:

I don't see any code difference between Orchard 1.7.2 and 1.8 for Orchard.Indexing module, especially related to this issue.

orchardbot commented 10 years ago

MrGenius commented:

I just found a repro steps for this problem. I would like to get experts feedback on this issue.

Repro Steps:

  1. Make sure you have a huge collection of content items to be indexed
  2. Rebuilding index under Admin/Indexing page or do a azure deployment both would trigger the the same

My observation about the issue:

Code block from Src\Orchard.Web\Modules\Orchard.Indexing\Services\UpdateIndexScheduler.cs

public void UpdateIndex(string indexName) {
            if(_indexingTaskExecutor.Value.UpdateIndexBatch(indexName)) {
                Schedule(indexName);
            }
        }

Assume I have a database with 10,000 indexable content items. The method indexingTaskExecutor.Value.UpdateIndexBatch(indexName) is going to return true for every batch. This method is not going to return false until it indexes all of them. That is 10,000/50=200 cycles of indexing tasks.

This means DefaultOrchardHost.EndRequest is not going to return until it finishes indexing all the content items.

orchardbot commented 10 years ago

MrGenius commented:

This is how I solved it.

public void UpdateIndex(string indexName)
        {
            if (_indexingTaskExecutor.Value.UpdateIndexBatch(indexName))
            {
                UpdateIndex(indexName);
            }
        }

This might not be the best solution considering how many times it get can executed in the worst case. But it will not hold the httprequest.

orchardbot commented 9 years ago

@sebastienros commented:

The issue has been opened, but I don't think the suggested fix is correct.

Piedone commented 9 years ago

I don't think the fix is correct either. Instead what we could not call UpdateIndex() at all when Rebuild is clicked: IndexingBackgroundTask fires such calls periodically anyway. You may already notice that we're back to the same problem: now the background task will run as long as not all the items were processed. The reason this is not happening now is because #4666 is not fixed yet, but it will be.

Maybe a solution would be that UpdateIndex() should only call UpdateIndexBatch() once?

@sebastienros what do you think?