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

Primary keys are required in MySQL DB cluster #21086

Closed
aqos156 opened this issue Sep 6, 2022 · 13 comments · Fixed by #21721 or #23605
Closed

Primary keys are required in MySQL DB cluster #21086

aqos156 opened this issue Sep 6, 2022 · 13 comments · Fixed by #21721 or #23605
Labels
issue/workaround it is or has a workaround type/bug
Milestone

Comments

@aqos156
Copy link

aqos156 commented Sep 6, 2022

Description

We use a hosted MySQL DB cluster service on digital ocean which requires that all tables have primary keys. Since version 1.17 there are tables without primary keys. This has happened once already in #16802.

Are you able to fix this regression so we are able to update to the newest version? Thank you for your time.

Gitea Version

1.16.9 (trying to update to 1.17)

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

https://gist.github.com/aqos156/b9b9b9f097ab926bec9be84fb34a1910

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Docker on swarm

Database

MySQL

@aqos156
Copy link
Author

aqos156 commented Oct 26, 2022

This problem is still persisting and relevant for us. We are unable to upgrade to the newest version.

@lunny
Copy link
Member

lunny commented Oct 27, 2022

ForeignReference table needs a primary key

@lunny lunny added this to the 1.17.4 milestone Oct 27, 2022
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 27, 2022

Is the table really working for some purpose now? Or how is it planned to be used in the future?

If no use at the moment and nobody know how to use it properly in the future, then it could be dropped.

And add it back when it's really necessary and needed.

@lunny
Copy link
Member

lunny commented Nov 9, 2022

Is the table really working for some purpose now? Or how is it planned to be used in the future?

If no use at the moment and nobody know how to use it properly in the future, then it could be dropped.

And add it back when it's really necessary and needed.

Since it's on v1.17, we cannot add migrations, both adding primary key or removing the tables cannot be done automatically.

@wolfogre
Copy link
Member

Is the table really working for some purpose now? Or how is it planned to be used in the future?
If no use at the moment and nobody know how to use it properly in the future, then it could be dropped.
And add it back when it's really necessary and needed.

Since it's on v1.17, we cannot add migrations, both adding primary key or removing the tables cannot be done automatically.

Maybe we can add a migration to add the primary key on v1.19, and move this issue to 1.19 milestone, I think it's better than endless waiting.

Then I will add a unit test to make sure a table without a primary key does not appear again, what the #21721 is trying to do.

And after that, we can discuss whether the table is necessary or not, and how to avoid adding unnecessary tables.

What do you think?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 17, 2022

I would say that the table is only used by dead code. Instead of adding a migration for primary key, adding a migration to drop it also seems to be an option (if no maintainer really knows how it works)

@lunny
Copy link
Member

lunny commented Nov 17, 2022

I would say that the table is only used by dead code. Instead of adding a migration for primary key, adding a migration to drop it also seems to be an option (if no maintainer really knows how it works)

It's expected to be used in future federation PRs.

@wxiaoguang
Copy link
Contributor

Yup, I know it was expected to be used in future federation PRs. But the question is how it works?

For my understanding, the ForeignIndex is used to store the federated Gitea's issue index. Does it really work for federated instances?

For example, suppose you have a repo-a with issues in Gitea Instance A, then you federate the repo-a to repo-b on Gitea Instance B, then you federate repo-b to repo-c on Gitea Instance C. If Instance B closes and quits the federation, how could you make the make repo-c sync(re-federate) with repo-a?

  • The issues in repo-a could be : (index=1), (index=2)
  • The issues in repo-b could be : (index=1, foreign_index=1(a)), (index=2), (index=3, foreign_index=2(a))
  • The issues in repo-c could be : (index=1, foreign_index=1(b)), (index=2, foreign_index=2(b)), (index=3, foreign_index=3(b))

How could the instances re-federate if repo-b disappears?

@lunny
Copy link
Member

lunny commented Dec 12, 2022

I think this cannot be fixed automatically in v1.17/v1.18, users could add a id column with autoincrement and primary key manually.

@lunny lunny modified the milestones: 1.17.4, 1.19.0 Dec 12, 2022
@lunny lunny added the issue/workaround it is or has a workaround label Dec 12, 2022
@aqos156
Copy link
Author

aqos156 commented Dec 22, 2022

So should we wait for the 1.19 release?

lunny pushed a commit that referenced this issue Dec 23, 2022
Some dbs require that all tables have primary keys, see
- #16802
- #21086

We can add a test to keep it from being broken again.

Edit:

~Added missing primary key for `ForeignReference`~ Dropped the
`ForeignReference` table to satisfy the check, so it closes #21086.

More context can be found in comments.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: zeripath <art27@cantab.net>
@Valinwolf
Copy link

I would just like to mention that when upgrading from an older version to 1.19.0, it still presents migration issues in regards to creating the ForeignReference table. Are there any guides to manually migrating my install?

@lunny
Copy link
Member

lunny commented Mar 20, 2023

The old related migrations should be also dropped.

@Valinwolf
Copy link

Valinwolf commented Mar 20, 2023

I just attempted to upgrade from a pre-1.17 version (I think) to 1.19 and I had errors about the ForeignReference table not having a primary key and couldn't be created. However, I managed to fix it by looking at my DB cluster query log and copying the query to insert an extra column so that it could move on to the next migration stage.

EDIT: After moving on from ForeignReference, I also found that ConfidentialClient lacked a primary key and was able to fix that by inserting a PRIMARY key column.

MySQL modified query:

CREATE TABLE IF NOT EXISTS `foreign_reference` (`entry_id` BIGINT(20) PRIMARY KEY, `repo_id` BIGINT(20) NULL, `local_index` BIGINT(20) NULL, `foreign_index` VARCHAR(255) NULL, `type` VARCHAR(16) NULL) DEFAULT CHARSET utf8 ROW_FORMAT=DYNAMIC;
CREATE TABLE IF NOT EXISTS `o_auth2_application` (`entry_id` BIGINT(20) PRIMARY KEY, `confidential_client` TINYINT(1) DEFAULT TRUE NOT NULL) DEFAULT CHARSET utf8 ROW_FORMAT=DYNAMIC;

zeripath pushed a commit that referenced this issue Mar 23, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Mar 23, 2023
jolheiser pushed a commit that referenced this issue Mar 24, 2023
Backport #23605 by @wolfogre

Fix
#21086 (comment)

Related to #21721

Co-authored-by: Jason Song <i@wolfogre.com>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/workaround it is or has a workaround type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants