Deleting repos deletes from disk but not database #385

New Issue

GiteaMirror · 2025-11-02T03:21:08-06:00

GiteaMirror commented

2025-11-02 03:21:08 -06:00

Originally created by @morrildl on GitHub (Feb 22, 2017).

I created some repos as a normal (non-admin) user, which I can push to, etc. However I cannot delete them.

If I attempt to, on the "type the name of the repo to confirm" screen, it hangs forever when I click "Confirm".

On disk, the physical repo directory is deleted. However, the entries for the deleted repos remain on dashboard. If you click one you get a 500.

Attached xorm.txt is a snippet from xorm.log of all entries that are printed after clicking the button.

Primary server log (even at Trace level) shows only this:

[Macaron] 2017-02-22 14:35:29: Started POST /morrildl/lol/settings for 172.17.0.1

I don't see any other relevant log output. Database is Postgres 9.6.

Originally created by @morrildl on GitHub (Feb 22, 2017). I created some repos as a normal (non-admin) user, which I can push to, etc. However I cannot delete them. If I attempt to, on the "type the name of the repo to confirm" screen, it hangs forever when I click "Confirm". On disk, the physical repo directory is deleted. However, the entries for the deleted repos remain on dashboard. If you click one you get a 500. Attached [xorm.txt](https://github.com/go-gitea/gitea/files/795024/xorm.txt) is a snippet from `xorm.log` of all entries that are printed after clicking the button. Primary server log (even at Trace level) shows only this: [Macaron] 2017-02-22 14:35:29: Started POST /morrildl/lol/settings for 172.17.0.1 I don't see any other relevant log output. Database is Postgres 9.6.

GiteaMirror added the type/bug label 2025-11-02 03:21:08 -06:00

GiteaMirror closed this issue

2025-11-02 03:21:08 -06:00

GiteaMirror commented

2025-11-02 03:21:09 -06:00

@morrildl commented on GitHub (Feb 23, 2017):

So after adding a few log.Debug()s I've determined it's wedging at https://github.com/go-gitea/gitea/blob/master/models/repo.go#L421 in DeleteWiki().

That xorm Delete() operation is resulting in this query:

2017/02/22 14:35:29 [I] [sql] DELETE FROM "repo_unit" WHERE repo_id = $1 AND type = $2 [args] [5 6]

However, earlier, this query is run in the same transaction:

2017/02/22 14:35:29 [I] [sql] DELETE FROM "repo_unit" WHERE repo_id = $1 [args] [5]

This doesn't seem like it should result in a deadlock since the second delete is a more-specific form of the first, and is within the same transaction, so as far as I can see it should be a no-op.

But I'm totally unfamiliar with xorm, so I don't really know whether it's xorm, the Go driver for PostgreSQL, the PG transaction engine, or what.

As a side note, the actual disk files for the repo, attachments, and wiki get deleted via calls to RemoveAllWithNotice() from within the middle of the code building up the transaction. This seems perhaps not quite ideal -- it might be better to store the list of files to be removed, and only proceed with deleting them once the transaction is complete.

@morrildl commented on GitHub (Feb 23, 2017): So after adding a few `log.Debug()`s I've determined it's wedging at https://github.com/go-gitea/gitea/blob/master/models/repo.go#L421 in DeleteWiki(). That xorm Delete() operation is resulting in this query: 2017/02/22 14:35:29 [I] [sql] DELETE FROM "repo_unit" WHERE repo_id = $1 AND type = $2 [args] [5 6] However, earlier, this query is run in the same transaction: 2017/02/22 14:35:29 [I] [sql] DELETE FROM "repo_unit" WHERE repo_id = $1 [args] [5] This doesn't seem like it should result in a deadlock since the second delete is a more-specific form of the first, and is within the same transaction, so as far as I can see it should be a no-op. But I'm totally unfamiliar with xorm, so I don't really know whether it's xorm, the Go driver for PostgreSQL, the PG transaction engine, or what. As a side note, the actual disk files for the repo, attachments, and wiki get deleted via calls to `RemoveAllWithNotice()` from within the middle of the code building up the transaction. This seems perhaps not quite ideal -- it might be better to store the list of files to be removed, and only proceed with deleting them once the transaction is complete.

GiteaMirror commented

2025-11-02 03:21:09 -06:00

@morrildl commented on GitHub (Feb 23, 2017):

So in repo.go I made this change:

--- a/models/repo.go
+++ b/models/repo.go
@@ -1572,7 +1572,7 @@ func DeleteRepository(uid, repoID int64) error {
                return err
        }
 
-       if _, err = sess.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil {
+       if _, err = sess.Where("repo_id = ?", repoID).And("type = ?", UnitTypeCode).Delete(new(RepoUnit)); err != nil {
                return err
        }

That is, I altered the first Delete() to constrain strictly to UnitTypeRepo, so that it is no longer a "superset" of the later DeleteWiki() query. With this change, the delete operation completes successfully. (It also works if you just comment out line 421, above.)

I doubt this is a correct fix though since it looks like it would leave a lot of potentially orphaned rows in the DB. But at least it confirms that something somewhere highly dislikes the more specific delete following the more general one.

@morrildl commented on GitHub (Feb 23, 2017): So in `repo.go` I made this change: --- a/models/repo.go +++ b/models/repo.go @@ -1572,7 +1572,7 @@ func DeleteRepository(uid, repoID int64) error { return err } - if _, err = sess.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil { + if _, err = sess.Where("repo_id = ?", repoID).And("type = ?", UnitTypeCode).Delete(new(RepoUnit)); err != nil { return err } That is, I altered the first `Delete()` to constrain strictly to `UnitTypeRepo`, so that it is no longer a "superset" of the later `DeleteWiki()` query. With this change, the delete operation completes successfully. (It also works if you just comment out line 421, above.) I doubt this is a correct fix though since it looks like it would leave a lot of potentially orphaned rows in the DB. But at least it confirms that something somewhere highly dislikes the more specific delete following the more general one.

GiteaMirror commented

2025-11-02 03:21:09 -06:00

@lunny commented on GitHub (Feb 23, 2017):

It's strange

@lunny commented on GitHub (Feb 23, 2017): It's strange

GiteaMirror commented

2025-11-02 03:21:09 -06:00

@morrildl commented on GitHub (Feb 23, 2017):

This is as far as I can take this today, but if anyone wants to poke at this, here is how I set up a Postgres 9.6 DB to test against, using Docker:

docker pull postgres:9.6
docker pull busybox
docker run --name pgsql-data -v /opt/data/pgsql/etc/postgresql-common:/etc/postgresql-common -v /opt/data/pgsql/var/log/postgresql:/var/log/postgresql -v /opt/data/pgsql/var/lib/postgresql/data:/var/lib/postgresql/data busybox true
docker create --restart=always -v /etc/localtime:/etc/localtime:ro --name pgsql -e POSTGRES_PASSWORD=asdffdsa --volumes-from pgsql-data postgres:9.6
docker start pgsql && sleep 5
sudo mkdir -p /opt/data/gitea
docker run --rm -ti --link pgsql -e PGPASSWORD=asdffdsa postgres:9.6 \
    psql -h pgsql -U postgres \
        -c "create role gitea encrypted password 'gitea' nosuperuser createdb createrole inherit login"
docker run --rm -ti --link pgsql -e PGPASSWORD=asdffdsa postgres:9.6 \
    createdb -h pgsql -U postgres --owner gitea --encoding utf8 gitea
TAGS="bindata" make docker
docker run --name gitea-data -v /opt/data/gitea:/data busybox true
docker create --restart always --link pgsql \
    -p 127.0.0.1:9030:3000 -p 127.0.0.1:9999:22 \
    --volumes-from gitea-data -v /etc/localtime:/etc/localtime:ro \
    --name gitea gitea/gitea && \
docker start gitea

@morrildl commented on GitHub (Feb 23, 2017): This is as far as I can take this today, but if anyone wants to poke at this, here is how I set up a Postgres 9.6 DB to test against, using Docker: docker pull postgres:9.6 docker pull busybox docker run --name pgsql-data -v /opt/data/pgsql/etc/postgresql-common:/etc/postgresql-common -v /opt/data/pgsql/var/log/postgresql:/var/log/postgresql -v /opt/data/pgsql/var/lib/postgresql/data:/var/lib/postgresql/data busybox true docker create --restart=always -v /etc/localtime:/etc/localtime:ro --name pgsql -e POSTGRES_PASSWORD=asdffdsa --volumes-from pgsql-data postgres:9.6 docker start pgsql && sleep 5 sudo mkdir -p /opt/data/gitea docker run --rm -ti --link pgsql -e PGPASSWORD=asdffdsa postgres:9.6 \ psql -h pgsql -U postgres \ -c "create role gitea encrypted password 'gitea' nosuperuser createdb createrole inherit login" docker run --rm -ti --link pgsql -e PGPASSWORD=asdffdsa postgres:9.6 \ createdb -h pgsql -U postgres --owner gitea --encoding utf8 gitea TAGS="bindata" make docker docker run --name gitea-data -v /opt/data/gitea:/data busybox true docker create --restart always --link pgsql \ -p 127.0.0.1:9030:3000 -p 127.0.0.1:9999:22 \ --volumes-from gitea-data -v /etc/localtime:/etc/localtime:ro \ --name gitea gitea/gitea && \ docker start gitea

GiteaMirror commented

2025-11-02 03:21:10 -06:00

@lunny commented on GitHub (Feb 23, 2017):

I will test this.

@lunny commented on GitHub (Feb 23, 2017): I will test this.

GiteaMirror commented

2025-11-02 03:21:10 -06:00

@lunny commented on GitHub (Feb 24, 2017):

@morrildl please help to confirm #1044 fix this.

@lunny commented on GitHub (Feb 24, 2017): @morrildl please help to confirm #1044 fix this.

GiteaMirror commented

2025-11-02 03:21:10 -06:00

@morrildl commented on GitHub (Feb 24, 2017):

I confirm that #1044 fixes this.

Thanks for taking a look, it would have taken me forever to track that one down!

@morrildl commented on GitHub (Feb 24, 2017): I confirm that #1044 fixes this. Thanks for taking a look, it would have taken me forever to track that one down!

GiteaMirror referenced this issue

2025-11-02 11:44:36 -06:00

[PR #385] [MERGED] Update example install url from try.gogs.io to try.gitea.io #15349

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#385