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

Gitea shows outdated commit status with distributed DB backend (eg. TiDB) #30074

Closed
stevapple opened this issue Mar 25, 2024 · 0 comments · Fixed by #30076
Closed

Gitea shows outdated commit status with distributed DB backend (eg. TiDB) #30074

stevapple opened this issue Mar 25, 2024 · 0 comments · Fixed by #30076
Labels

Comments

@stevapple
Copy link
Contributor

stevapple commented Mar 25, 2024

Description

Gitea doesn’t always show the latest commit status (i.e. being outdated) if you use some distributed database as backend. An auto_increment primary key, though being expected to bump at a step of 1 at every INSERT, may be designed to bump independently on each nodes.

We should update commit_status.go to use the (max(`index`), repo_id, sha) triple to determine the latest commit status, instead of relying on max(id).

Gitea Version

1.21.9

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

I’m running Gitea on a Kubernetes cluster, which also hosts Woodpecker CI v2.3.0 and TiDB v7.5.1. Gitea uses TiDB (through mysql) as backend and Woodpecker uses Gitea as forge.

The problem was originally identified as “randomly dropping commit status on either Woodpecker, Gitea and TiDB side”, but after digging into the database it turns out that all commit statuses were there.

Database

MySQL/MariaDB

lunny pushed a commit that referenced this issue Mar 28, 2024
…30076)

This PR replaces the use of `max( id )`, and instead using ``max(
`index` )`` for determining the latest commit status. Building business
logic over an `auto_increment` primary key like `id` is risky and
there’re already plenty of discussions on the Internet.

There‘s no guarantee for `auto_increment` values to be monotonic,
especially upon failures or with a cluster. In the specific case, we met
the problem of commit statuses being outdated when using TiDB as the
database. As [being
documented](https://docs.pingcap.com/tidb/stable/auto-increment),
`auto_increment` values assigned to an `insert` statement will only be
monotonic on a per server (node) basis.

Closes #30074.
stevapple added a commit to stevapple/gitea that referenced this issue Mar 28, 2024
…o-gitea#30076)

This PR replaces the use of `max( id )`, and instead using ``max(
`index` )`` for determining the latest commit status. Building business
logic over an `auto_increment` primary key like `id` is risky and
there’re already plenty of discussions on the Internet.

There‘s no guarantee for `auto_increment` values to be monotonic,
especially upon failures or with a cluster. In the specific case, we met
the problem of commit statuses being outdated when using TiDB as the
database. As [being
documented](https://docs.pingcap.com/tidb/stable/auto-increment),
`auto_increment` values assigned to an `insert` statement will only be
monotonic on a per server (node) basis.

Closes go-gitea#30074.
silverwind pushed a commit that referenced this issue Mar 28, 2024
…30076) (#30155)

Backport #30076.

This PR replaces the use of `max( id )`, and instead using ``max(
`index` )`` for determining the latest commit status. Building business
logic over an `auto_increment` primary key like `id` is risky and
there’re already plenty of discussions on the Internet.

There‘s no guarantee for `auto_increment` values to be monotonic,
especially upon failures or with a cluster. In the specific case, we met
the problem of commit statuses being outdated when using TiDB as the
database. As [being
documented](https://docs.pingcap.com/tidb/stable/auto-increment),
`auto_increment` values assigned to an `insert` statement will only be
monotonic on a per server (node) basis.

Closes #30074.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant