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: Dont lock datastore lock within lock 3 #1273

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Mar 31, 2023

Relevant issue(s)

Resolves #1154

Description

An alternative solution to #1261 that removes the shared lock completely, at the cost of a pointer in the txn tree and a mutex and bool on the txn.

@AndrewSisley AndrewSisley added bug Something isn't working area/datastore Related to the datastore / storage engine system action/no-benchmark Skips the action that runs the benchmark. labels Mar 31, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.5 milestone Mar 31, 2023
@AndrewSisley AndrewSisley requested a review from a team March 31, 2023 22:14
@AndrewSisley AndrewSisley self-assigned this Mar 31, 2023
@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #1273 (036d7c5) into develop (561921a) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1273      +/-   ##
===========================================
- Coverage    70.27%   70.23%   -0.04%     
===========================================
  Files          184      184              
  Lines        17791    17798       +7     
===========================================
- Hits         12502    12500       -2     
- Misses        4340     4347       +7     
- Partials       949      951       +2     
Impacted Files Coverage Δ
datastore/memory/memory.go 95.83% <100.00%> (+0.13%) ⬆️
datastore/memory/txn.go 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@AndrewSisley AndrewSisley changed the title fix: Dont lock datastore lock within lock fix: Dont lock datastore lock within lock 3 Mar 31, 2023
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I1154-fix-ds-deadlock-3 branch from 734c9fd to 2bf7e48 Compare April 3, 2023 21:28
Can result in a deadlock.
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I1154-fix-ds-deadlock-3 branch from 2bf7e48 to 036d7c5 Compare April 3, 2023 21:30
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

I can get behind this change 👍

@AndrewSisley AndrewSisley merged commit 371a43f into develop Apr 4, 2023
@AndrewSisley AndrewSisley deleted the sisley/fix/I1154-fix-ds-deadlock-3 branch April 4, 2023 01:14
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/datastore Related to the datastore / storage engine system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky test - TestP2PWithMultipleDocumentUpdatesPerNodeWithP2PCollection
2 participants