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

Added: A Writer Lock for SqliteDataStore #1090

Merged
merged 1 commit into from
Mar 20, 2024
Merged

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Mar 20, 2024

This PR adds a simple writer lock into SqliteDataStore, such that concurrent access to the database doesn't result in failure.

Although we will deprecate the data store I also (just in case) timed the performance of real workloads against this PR. Difference is negligible.

For more details, see #1089


fixes #1089

@Sewer56 Sewer56 added Bug Something isn't working Tech: data-store This is related to the Data Store. labels Mar 20, 2024
@Sewer56 Sewer56 requested a review from a team March 20, 2024 06:13
@Sewer56 Sewer56 self-assigned this Mar 20, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.59%. Comparing base (3a826d4) to head (34bd30f).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1090      +/-   ##
==========================================
- Coverage   54.59%   54.59%   -0.01%     
==========================================
  Files         669      669              
  Lines       22024    22037      +13     
  Branches     1706     1706              
==========================================
+ Hits        12023    12030       +7     
- Misses       9608     9612       +4     
- Partials      393      395       +2     
Flag Coverage Δ
Linux 53.91% <100.00%> (-0.01%) ⬇️
Windows 53.83% <100.00%> (-0.02%) ⬇️
clean_environment_tests 54.58% <100.00%> (+<0.01%) ⬆️
macOS 53.39% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/NexusMods.DataModel/FileHashCache.cs 96.00% <ø> (ø)
src/NexusMods.DataModel/SqliteDataStore.cs 78.85% <100.00%> (+0.96%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

Seems ok to me

@Sewer56 Sewer56 merged commit 06a338f into main Mar 20, 2024
5 checks passed
Copy link
Member

@erri120 erri120 left a comment

Choose a reason for hiding this comment

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

Task<long> PutRaw should be removed as well, it's unused.

src/NexusMods.DataModel/SqliteDataStore.cs Show resolved Hide resolved
src/NexusMods.DataModel/SqliteDataStore.cs Show resolved Hide resolved
src/NexusMods.DataModel/FileHashCache.cs Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Tech: data-store This is related to the Data Store.
4 participants