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.

Sign up for GitHub

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

Jump to bottom

[stable27] fix(sync): prevent race condition by relying on autoincrement #4970

Merged
merged 2 commits into from
Nov 8, 2023
Merged

[stable27] fix(sync): prevent race condition by relying on autoincrement #4970

merged 2 commits into from
Nov 8, 2023

Commits on Nov 8, 2023

  1. fix(sync): prevent race condition by relying on autoincrement 

    Prevent a possible race condition when two clients add steps at the same time.

See #4600.

Rely on the autoincrementing id in order to provide a canonical order
that steps can be retrieved in.

When two clients push steps at the same time
the entries receive destinct ids that increment.
So if another client fetches steps in between
it will see the smaller id as the version of the fetched step
and fetch the other step later on.

Transition:
In the future we can drop the version column entirely
but currently there are still steps stored in the database
that make use of the old column.
So we need to transition away from that.

In order to find entries that are newer than version x
we select those that have both a version and an id larger than x.

Entries of the new format are newer than any entry of the old format.
So we set their version to the largest possible value.
This way they will always fulfill the version condition
and the condition on the id is more strict and therefore effective.

For the old format the version will be smaller than the id
as it's incremented per document while the id is unique accross documents.
Therefore the version condition is the more strict one and effective.

The only scenario where the version might be larger than the id
would be if there's very few documents in the database
and they have had a lot of steps stored in single database entries.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Jonas <jonas@freesources.org>
    @max-nextcloud @mejo-
    max-nextcloud authored and mejo- committed Nov 8, 2023
    Configuration menu
    Copy the full SHA
    e2d7973 View commit details
    Browse the repository at this point in the history

  2. fix(Step): Use largest possible 32-bit int value for transition 

    The value used before (largest possible MySQL BIGINT value) was too big
for PHP int. Since we still support 32-bit platforms on Nextcloud, let's
stick to the largest possible 32-bit PHP integer value.

Besides, setting the value as default for `Step::version` doesn't work
as `QBMapper->insert()` doesn't recognize the `version` field as changed
in that case. So let's default to `0` again and set it using
`Step->setVersion()` later.

Signed-off-by: Jonas <jonas@freesources.org>
    @mejo-
    mejo- committed Nov 8, 2023
    Configuration menu
    Copy the full SHA
    7d2eb43 View commit details
    Browse the repository at this point in the history