Skip to content
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

Fix #115 Concurrency issue when adding documents. #116

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

niemyjski
Copy link
Member

@niemyjski niemyjski commented Dec 23, 2024

Fixes #115

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.set_Item(TKey key, TValue value)
   at Foundatio.Repositories.Elasticsearch.Configuration.DailyIndex.EnsureDateIndexAsync(DateTime utcDate) in /_/src/Foundatio.Repositories.Elasticsearch/Configuration/DailyIndex.cs:line 150
   at Foundatio.Repositories.Elasticsearch.ElasticRepositoryBase`1.IndexDocumentsAsync(IReadOnlyCollection`1 documents, Boolean isCreateOperation, ICommandOptions options) in /_/src/Foundatio.Repositories.Elasticsearch/Repositories/ElasticRepositoryBase.cs:line 1252
   at Foundatio.Repositories.Elasticsearch.ElasticRepositoryBase`1.AddAsync(IEnumerable`1 documents, ICommandOptions options) in /_/src/Foundatio.Repositories.Elasticsearch/Repositories/ElasticRepositoryBase.cs:line 93
   at Foundatio.Repositories.Elasticsearch.ElasticRepositoryBase`1.AddAsync(T document, ICommandOptions options) in /_/src/Foundatio.Repositories.Elasticsearch/Repositories/ElasticRepositoryBase.cs:line 67

@niemyjski niemyjski added the bug label Dec 23, 2024
@niemyjski niemyjski requested a review from ejsmith December 23, 2024 19:10
@niemyjski niemyjski self-assigned this Dec 23, 2024
include date on lock

Tweaked logic
@niemyjski niemyjski force-pushed the bugfix/race-condition-index-creation branch from 174add4 to 8b4b117 Compare December 23, 2024 20:40
protected async Task EnsureDateIndexAsync(DateTime utcDate)
{
utcDate = utcDate.Date;
if (_ensuredDates.ContainsKey(utcDate))
return;

await using var indexLock = await _ensureIndexLock.AcquireAsync($"Index:{GetVersionedIndex(utcDate)}", TimeSpan.FromMinutes(1)).AnyContext();
Copy link
Member Author

Choose a reason for hiding this comment

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

It kind of feels like we should be locking on the index name here, but this allows us to go wide. I'm wondering if we should also change this to a concurrent dictionary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant