-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[stable10] Protect group share concurrent updates with upsert #34769
Conversation
|
Codecov Report
@@ Coverage Diff @@
## stable10 #34769 +/- ##
==============================================
+ Coverage 64.19% 64.2% +<.01%
Complexity 19883 19883
==============================================
Files 1278 1278
Lines 76244 76246 +2
Branches 1291 1291
==============================================
+ Hits 48948 48950 +2
Misses 26917 26917
Partials 379 379
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## stable10 #34769 +/- ##
==============================================
+ Coverage 64.19% 64.2% +<.01%
Complexity 19883 19883
==============================================
Files 1278 1278
Lines 76244 76246 +2
Branches 1291 1291
==============================================
+ Hits 48948 48950 +2
Misses 26917 26917
Partials 379 379
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## stable10 #34769 +/- ##
==============================================
- Coverage 64.25% 64.24% -0.01%
- Complexity 19990 19993 +3
==============================================
Files 1281 1281
Lines 76663 76648 -15
Branches 1307 1307
==============================================
- Hits 49258 49243 -15
Misses 27024 27024
Partials 381 381
Continue to review full report at Codecov.
|
|
note that even if we only add a constraint without this fix, there is some probability of random error messages |
after discussing with @tomneedham transactions not a good idea as they could cause deadlocks
|
32d622f
to
29d53be
Compare
Upsert added and this is already covered by unit tests and seem to work. Let's see if it also works in a broader scope (integration/acceptance tests in CI). |
also had a quick test in the UI with a group share and renamed / unshared from self as recipient to test insert and update. the DB entries were updated fine. |
@tomneedham review upsert ? |
master port here: #34952 |
To avoid race conditions happening exactly between the select and insert/update.
29d53be
to
ee04539
Compare
adjusted to solve conflict, now that #34951 was merged |
Description
To avoid race conditions happening exactly between the select and
insert/update.
Related Issue
https://github.com/owncloud/enterprise/issues/3167 and #27990
Motivation and Context
How Has This Been Tested?
Automated tests
Screenshots (if appropriate):
Types of changes
Checklist:
Open tasks: