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

handleCreateError()] [E] CreatePost: database table is locked: repository' #1398

Closed
2 of 7 tasks
typeless opened this issue Mar 28, 2017 · 17 comments
Closed
2 of 7 tasks
Labels

Comments

@typeless
Copy link
Contributor

  • Gitea version (or commit ref):
    Gitea 1.10

  • Git version:
    it version 2.7.4

  • Operating system:
    "Ubuntu 16.04.2 LTS"

  • Database (use [x]):

    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:

    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:
    In log/gitea.log
    routers/repo/repo.go:103 handleCreateError()] [E] CreatePost: database table is locked: repository

Description

Creating a new repo would occasionally return HTTP code 500.
...

@lunny lunny added the type/bug label Mar 28, 2017
@lunny lunny added this to the 1.2.0 milestone Mar 28, 2017
@lunny
Copy link
Member

lunny commented Mar 31, 2017

I cannot reproduce in master version

@typeless
Copy link
Contributor Author

@lunny yes, this is not easy to reproduce. I'll provide more details (or solve it myself ) when I encounter the problem again.

@typeless
Copy link
Contributor Author

By the way, maybe many users accessing the server simultaneously can trigger it.

@lunny
Copy link
Member

lunny commented Mar 31, 2017

maybe

@typeless
Copy link
Contributor Author

Once the new integration test is merged, we can add certain stress tests for such scenario. I am also thinking about migrate all global maps to syncmap gradually. That makes sense because gitea is a highly concurrent application.

@plessbd
Copy link

plessbd commented Apr 3, 2017

Similar to #1434 and #1372
At least for #1372 it doesn't have to do with load as I was the only one using the system at that point.

@typeless
Copy link
Contributor Author

typeless commented Apr 5, 2017

The pattern used at https://github.com/go-gitea/gitea/blob/master/models/repo.go#L2000 is probably related.

See the pattern at the beginning of several related functions:

	if taskStatusTable.IsRunning(gitFsck) {
		return
	}
	taskStatusTable.Start(gitFsck)
	defer taskStatusTable.Stop(gitFsck)

It looks like the intention is to emulate some kind of mutex lock, but the problem is the read and lock is not combined into an atomic operation. They are separated into IsRunning() and Start() respectively. That could be problematic because there is no guarantee that, at the very moment after the call to IsRunning and before Start, another goroutine wouldn't be launched and calling into the same sequence.

@strk
Copy link
Member

strk commented Apr 5, 2017 via email

@typeless
Copy link
Contributor Author

typeless commented Apr 6, 2017

@strk I am still not sure that is the root cause of the problem despite it's not quite correct.

Before replacing it with something else, I would try to figure out what resources are shared and supposed to be concurrency-safe.

One thing does confuse me though. There are several tables of the database with duplicate information. For example, there is a watch_num column for each repository, but a table named watch has the the repo_id and watch_num mapping as well. As a result, there is a background goroutine for syncing the two, which I can not understand its purpose and think it'd better be simplified. See https://github.com/go-gitea/gitea/blob/master/models/repo.go#L2009. I guess that would probably be for performance reason, like speeding up some kind of queries. I am not familiar with database management, so please correct me if I am wrong.

@lunny
Copy link
Member

lunny commented Apr 6, 2017

I agree @typeless. We need a queue to do that jobs, and for SQLITE, we could add a config item to disable this check running and provide a button on admin panel to run it manually.

@typeless
Copy link
Contributor Author

typeless commented Apr 6, 2017

@lunny If possible (and reasonable), I'd like to eliminate those tables with redundant information and the 'syncing' thing altogether. I expect the queries serviced by those extra tables can be done with some slightly sophisticated SQLs and probably indexes can help with performance if that really matters.

P.S. Otherwise, keeping the tables and removing the columns with the same information in the other tables.

@lunny
Copy link
Member

lunny commented Apr 20, 2017

Is this resolved already?

@typeless
Copy link
Contributor Author

I haven't seen the problem for a while after upgrade but I am not sure if it's 100% clean (there is a pending PR for issues probably related to this).

@lunny
Copy link
Member

lunny commented Apr 20, 2017

Which one? I find all the three links issues to this one are merged or closed.

@typeless
Copy link
Contributor Author

@lunny #1470

@lunny
Copy link
Member

lunny commented Jun 20, 2017

database lock is still exist on sqlite3 and move this to v1.3

@lunny lunny modified the milestones: 1.3.0, 1.2.0 Jun 20, 2017
@lunny
Copy link
Member

lunny commented Jun 23, 2017

continue via #2040

@lunny lunny closed this as completed Jun 23, 2017
@lunny lunny removed this from the 1.3.0 milestone Jun 23, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants