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

Lock/unlock tables when editing entries #2585

Merged
merged 1 commit into from May 31, 2016

Conversation

michael-e
Copy link
Member

When editing an entry, Symphony will iterate over all fields, delete existing data first, then insert a new row for the field's data. So there will always be a time slot without field data. If multiple processes edit the same entry, this means a race condition that may lead to data being corrupted in several ways (also depending on the field type). Locking the table solves the issue.

Fixes #2472

When editing an entry, Symphony will iterate over all fields, delete existing data first, then insert a new row for the field's data. So there will always be a time slot without field data. If multiple processes edit the same entry, this means a race condition that may lead to data being corrupted in several ways (also depending on the field type). Locking the table solves the issue.

Fixes symphonycms#2472
}

Symphony::Database()->insert($fields, 'tbl_entries_data_' . $field_id);
if ($did_lock) {
Copy link
Member

Choose a reason for hiding this comment

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

This block should be in a finally statement to be 100% kasher (if I am not wrong?)

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 discussed that some months ago (#2472 (comment)); the issue is that finally requires PHP 5.5+.

Copy link
Member

Choose a reason for hiding this comment

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

I always forget about that. Thanks!

@nitriques
Copy link
Member

Thanks Michael. See my comments!

@nitriques
Copy link
Member

I merge ASAP then!

@nitriques nitriques added this to the 2.7.0 milestone May 31, 2016
@nitriques nitriques self-assigned this May 31, 2016
@nitriques nitriques merged commit 4b5067f into symphonycms:2.7.x May 31, 2016
@michael-e michael-e mentioned this pull request Jul 19, 2016
@michael-e michael-e deleted the fix-2472 branch November 4, 2016 09:37
nitriques added a commit to DeuxHuitHuit/symphonycms that referenced this pull request May 17, 2017
58bc57f added DISTINCT and LIMIT clauses to sub-queries used for sorts.
The sub-query MUST return only one row, and (sometimes) it does not.
See <INSERT REF HERE>

As demonstrated by @michael-e, doing this does not suffice, since the
php code also needs to change (or we would need to add more DISTINCT
which is not better).

The new table locking mechanism (symphonycms#2585) + making sure we update existing sites
that are missing the unique key should prevent (hopefully) any data
corroption before it can happen.
jensscherbl pushed a commit that referenced this pull request May 28, 2017
When editing an entry, Symphony will iterate over all fields, delete existing data first, then insert a new row for the field's data. So there will always be a time slot without field data. If multiple processes edit the same entry, this means a race condition that may lead to data being corrupted in several ways (also depending on the field type). Locking the table solves the issue.

Fixes #2472
jensscherbl pushed a commit that referenced this pull request May 28, 2017
58bc57f added DISTINCT and LIMIT clauses to sub-queries used for sorts.
The sub-query MUST return only one row, and (sometimes) it does not.
See <INSERT REF HERE>

As demonstrated by @michael-e, doing this does not suffice, since the
php code also needs to change (or we would need to add more DISTINCT
which is not better).

The new table locking mechanism (#2585) + making sure we update existing sites
that are missing the unique key should prevent (hopefully) any data
corroption before it can happen.
nitriques pushed a commit that referenced this pull request Jun 16, 2017
When editing an entry, Symphony will iterate over all fields, delete existing data first, then insert a new row for the field's data. So there will always be a time slot without field data. If multiple processes edit the same entry, this means a race condition that may lead to data being corrupted in several ways (also depending on the field type). Locking the table solves the issue.

Fixes #2472

Picked from 4b5067f
nitriques added a commit that referenced this pull request Jun 16, 2017
58bc57f added DISTINCT and LIMIT clauses to sub-queries used for sorts.
The sub-query MUST return only one row, and (sometimes) it does not.
See <INSERT REF HERE>

As demonstrated by @michael-e, doing this does not suffice, since the
php code also needs to change (or we would need to add more DISTINCT
which is not better).

The new table locking mechanism (#2585) + making sure we update existing sites
that are missing the unique key should prevent (hopefully) any data
corroption before it can happen.

Picked from 86717ba
nitriques pushed a commit that referenced this pull request Jun 16, 2017
When editing an entry, Symphony will iterate over all fields, delete existing data first, then insert a new row for the field's data. So there will always be a time slot without field data. If multiple processes edit the same entry, this means a race condition that may lead to data being corrupted in several ways (also depending on the field type). Locking the table solves the issue.

Fixes #2472

Picked from 4b5067f
nitriques added a commit that referenced this pull request Jun 16, 2017
58bc57f added DISTINCT and LIMIT clauses to sub-queries used for sorts.
The sub-query MUST return only one row, and (sometimes) it does not.
See <INSERT REF HERE>

As demonstrated by @michael-e, doing this does not suffice, since the
php code also needs to change (or we would need to add more DISTINCT
which is not better).

The new table locking mechanism (#2585) + making sure we update existing sites
that are missing the unique key should prevent (hopefully) any data
corroption before it can happen.

Picked from 86717ba
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants