Skip to content

Changing the computation of next issue index #4084

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

Closed
2 of 7 tasks
eapetitfils opened this issue May 30, 2018 · 6 comments
Closed
2 of 7 tasks

Changing the computation of next issue index #4084

eapetitfils opened this issue May 30, 2018 · 6 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality

Comments

@eapetitfils
Copy link

eapetitfils commented May 30, 2018

  • Gitea version (or commit ref): 1.4.1
  • Git version: 2.7.4
  • Operating system: Ubuntu 16.04
  • Database:
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

While looking at importing issues from Redmine, I noticed a corner case that could cause problems when the database is tempered with:

When creating a new issue, the issue number is computed with opts.Repo.NextIssueIndex() which returns the sum of issues and pull requests stored in the repository table. If the numbering of issues and pull request is not continuous, this could lead to trying to create an issue with an index already existing,

Would it be better to generate the new index number by using the maximum index in the issue table?

The index column is indexed so there should not be a significant impact and would prevent any issue even if someone tempered a bit with the database. And doing it relying on the atomicity of the database could prevent the need for a MUTEX as said in the code.

@sapk
Copy link
Member

sapk commented May 30, 2018

Good idea to be able to import issues from other system and ease the process of migration.
For the mutex, it will still be needed as the goal is to not have a race when two issue are created at the same time before any of them are yet written to the database. The last to be recorded would fail normally but it would be good to prevent that.

@lunny lunny added the type/bug label May 31, 2018
@lunny lunny added this to the 1.x.x milestone May 31, 2018
@eapetitfils
Copy link
Author

eapetitfils commented May 31, 2018

Importing the issues might be a bit tricky to add. There are requests for github in #3757 #479 #1445 and I understand why this should not be part of Gitea: it opens the Pandora box and will turn out to be very tricky to maintain due to the various ways the issues are indexed on these systems.

And even then, there will be people like me who are trying to merge repositories where the commits are referencing some issues stored in Redmine AND other issues stored in Gitea. I suspect I will do a lot manual tweaks in the end.

Also I am not sure if this can be classified as a bug. This would be closer to an enhancement as the code is working as is now, what I propose is mostly increasing the robustness.

@lunny lunny added type/enhancement An improvement of existing functionality and removed type/bug labels Jun 1, 2018
@stale
Copy link

stale bot commented Jan 25, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Jan 25, 2019
@eapetitfils
Copy link
Author

Do we want to include this in the 2.x.x milestone?

@lunny lunny added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented and removed issue/stale labels Feb 7, 2019
@lunny
Copy link
Member

lunny commented Jan 25, 2020

I think this has been resolved?

@guillep2k
Copy link
Member

guillep2k commented Jan 25, 2020

@lunny yes. Index numbers are currently calculated from the higher existing, so "holes" are not a problem anymore.

@lunny lunny closed this as completed Jan 25, 2020
@lunny lunny removed this from the 1.x.x milestone Jan 25, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality
Projects
None yet
Development

No branches or pull requests

4 participants