Skip to content

Conversation

@MikeAlhayek
Copy link
Member

No description provided.

@MikeAlhayek MikeAlhayek requested a review from gvkries September 24, 2025 15:06
await context.WaitForHttpBackgroundJobsAsync(TestContext.Current.CancellationToken);

// Indexing may still be running, so we wait a bit to ensure the content item is indexed.
await Task.Delay(2000, TestContext.Current.CancellationToken);
Copy link
Member Author

@MikeAlhayek MikeAlhayek Sep 24, 2025

Choose a reason for hiding this comment

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

@gvkries can you think of a better way than delaying? This works locally but not here. guessing that the delay need to be much higher to pass the CLI

Copy link
Member

Choose a reason for hiding this comment

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

In theory, our waiting methods should now be reliable. Both WaitForOutstandingDeferredTasksAsync and WaitForHttpBackgroundJobsAsync take the active count of operations into account and complete as quickly as possible.

One possible issue might be the call pattern you're using. It looks like you're starting an HTTP background job from within a deferred action, which could be causing a race condition with the wait calls.

It might be worth considering a refactor to rely solely on HTTP background tasks. This could simplify the flow and reduce the potential for timing-related issues.

Copy link
Member Author

@MikeAlhayek MikeAlhayek Sep 25, 2025

Choose a reason for hiding this comment

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

The change in the ContentIndexProfileHandler has to be executed in a deferred task to ensure we first save the changes before triggering background.

If the background executor does it what should ExecuteAtEndOrRequest, this won't be an issue :)

The background job helper should have one method called ExecuteAsync and the existing one. The existing one should not be executed until all the deferred tasks are completed

Copy link
Member

Choose a reason for hiding this comment

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

Understood, and I think this should work as-is, it's not clear to me why waiting will fail. If a deferred task starts the HTTP job with ExecuteAfterEndOfRequestAsync, WaitForOutstandingDeferredTasksAsync should complete, but WaitForHttpBackgroundJobsAsync must wait until the indexing has been completed.

Copy link
Member Author

Choose a reason for hiding this comment

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

we'll it should but it is not. hopfully the logging you are adding in the other PR will give us more clues on why it is not

Copy link
Member

Choose a reason for hiding this comment

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

I've done some initial debugging and it appears the issue isn't with the second item being added, but rather that the initial item is missing. The one with the display text "Some sort of blogpost in a Query!" is present, but the other expected item is not.
Additionally, I'm encountering frequent ObjectDisposedException errors from the writer used by LuceneIndexManager. This suggests a possible concurrency issue. I haven’t had time to investigate this in depth yet, but it might be worth looking into thread safety or lifecycle management around the writer.

Copy link
Member

Choose a reason for hiding this comment

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

It appears the core issue isn't about waiting for background tasks to complete, but rather about concurrency in the indexing operations. Specifically, multiple HTTP background jobs are being triggered simultaneously:

  1. RebuildIndexStep
  2. ContentIndexProfileHandler
  3. ContentIndexInitializerService

Since these jobs run concurrently and without a guaranteed execution order, the Lucene index occasionally fails to include content items added by the recipe. The main culprit seems to be RebuildIndexStep, which resets and rebuilds the index in a way that may not be thread-safe:

await indexProfileManager.ResetAsync(index);
await indexProfileManager.UpdateAsync(index);
if (await indexManager.RebuildAsync(index))
{
    await indexProfileManager.SynchronizeAsync(index);
}

This sequence likely introduces race conditions, especially when other indexing tasks are running in parallel. Ensuring thread safety or introducing proper synchronization between these steps might help resolve the inconsistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants