Sometimes push to gitea will fail

[gslin]

• Gitea version (or commit ref): 1.6.2
• Git version: 2.20.1 (gitea side) & 2.7.4 (client)
• Operating system: Ubuntu 18.04.1 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: https://gist.github.com/gslin/1267f3f0fc3675f345a149246b9ce14a

Description

When I upgraded Gitea from 1.5.x to 1.6.0, sometimes git push would fail with different reasons. Usually rerun git push will be successful (but not always).

[lafriks]

Do you see any errors in gitea.log?

[gslin]

Yes, I updated it to https://gist.github.com/gslin/1267f3f0fc3675f345a149246b9ce14a and mostly it included "MirrorUpdate: invalid connection".

[lafriks]

This error is when connection to database is broken but it's hard to tell reason for this

[gslin]

I enabled log-error in MySQL to get error messages, and this seems like connection dropped randomly:

https://gist.github.com/gslin/c5aa707b7900ffe8223df47c878bf8ad

Since 1.5.x is fine, is MySQL library in 1.6.x different from 1.5.x?

[lunny]

The error is

retrieve protected branches information failed: Failed to update public key: EOF

[maxiv]

Have a same error even on navigation inside Gitea (1.6.3 and 1.7.0-rc2):

[...s/context/context.go:235 func1()] [E] GetUserById: invalid connection

In mysql.log:

Aborted connection 21 to db: 'projects_gitea' user: 'projects_gitea' host: 'localhost' (Got timeout reading communication packets)

[gslin]

I've reproduced this on https://test-gitea.404.tw, a brand new setup with Ubuntu 18.04 + Percona Server 5.7.24 + Gitea 1.6.4 + nginx 1.14.0. I would love to provide this instance for debugging.

[typeless]

Is it related to #5736?

[zeripath]

Possibly, however another option is that we're causing a deadlock in gitea serv when we check and update the public key to say it's used, mysql is then detecting the deadlock and just killing the connection.

https://github.com/go-gitea/gitea/blob/master/models/ssh_key.go#L503

Looking at that code if we really want to update the public key entry every time it's used, this should be done in a transaction.

[typeless]

@zeripath I see. I wonder if it's possible to replace the global x with context.Context one day.

[zeripath]

It's not definitely the issue - I'm just suspicious.

I think the global x is too tempting really. In other languages, they allow that style by adding threadlocals and a handler that opens a transaction per http request or alike. In go you can't set a threadlocal, and you're meant to put the context into every call. To get rid of the global x would mean restructuring all our functions to force the passing in of a session, as appropriate - thus it's not hidden anymore and anyone that uses them has to think about it.

[lunny]

@typeless I don't think the x is the problem. Even if you are using database/sql you will use db on all go routines. x will always new a session object so this should not a deadlock reason.

[typeless]

@lunny Not sure if I understand you correctly. But I was not talking about replacing the database engine of x with something else. Instead, I was talking about different approaches to passing the database handle like this is discussing.

My rule of thumb is that global variables make multi-threading harder. But I agree, this particular issue is not necessarily caused by it. I just imagined that, if the database connection handle were not global, would it be less error-prone in the first place?

[gslin]

@typeless I tried master version from https://dl.gitea.io/ (which already fixed #5736 issue) and this happened with git pull:

VERSION:
   a757920 built with go1.11.1 : bindata, sqlite, sqlite_unlock_notify

Gitea: Internal error
Failed to get repository: Failed to get repository: invalid connection
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

[typeless]

@gslin I grepped the codebase and found this https://github.com/go-gitea/gitea/blob/master/docs/content/doc/help/troubleshooting.en-us.md. It might be helpful.

Also, if this used to work, what is the working version?

[lunny]

@typeless that's a good article. It discussed serval different database codes organization methods.

[stale]

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.

[gslin]

The last version working without issue is 1.5.3.

[typeless]

@gslin I used to encounter a similar problem. It turned out that I have to create a team with proper access rights within the organization and add the accounts to it. The team feature was added in later versions. You could have a look at it.

Edit: This looks like a randomly happened problem. Probably not related to what I said.

[gslin]

My issue happens in all pages (randomly), including login page 500.

[typeless]

@gslin I suggest upgrading to a newer version to find if it would still happen.

[zeripath]

@gslin - we never did get to the bottom of this.

There are several possible causes for this issue:

1. Too many connections to the database at the same time.
2. Deadlocks are occurring as a result of errors in the way we're structuring transactions in Gitea
3. Deadlocks are occurring because of high contention and multiple changes at the same time but we're not managing this appropriately.
4. It's possible that deadlock killed connections are being placed in the pool or similarly very old connections are being placed in the pool which the db is later just dropping and we're not checking these are ready to be used again.

If its the first case, it should be fixable through already configurable by adding more connections to the pool and probably on the Mysql side of things.

2 and 3 are the main worries for me. I am not confident that we're doing things properly.

The final case would technically be a problem with Xorm.

I'm not sure how to proceed. First things first though - if you're still getting this - what version are you on? Could you test on the 1.8-rcs?

[bobemoe]

I've been researching this over at #6804, I think this may confirm option 4 from @zeripath above?