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

random database lock when sqlite database #2040

Closed
lunny opened this issue Jun 23, 2017 · 25 comments · Fixed by #2116 or #5144
Closed

random database lock when sqlite database #2040

lunny opened this issue Jun 23, 2017 · 25 comments · Fixed by #2116 or #5144
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug

Comments

@lunny
Copy link
Member

lunny commented Jun 23, 2017

This is a summary issue since there are so many issues to report the database lock when using sqlite. I will close all other issues and please continue in this issue.

@lunny
Copy link
Member Author

lunny commented Jun 23, 2017

And maybe we should pause make test-sqlite util this issue is resolved.

@typeless
Copy link
Contributor

Reference: https://sqlite.org/unlock_notify.html

@bkcsoft
Copy link
Member

bkcsoft commented Jun 28, 2017

@typeless Unless that is available in https://github.com/mattn/go-sqlite3 and https://github.com/go-xorm/xorm/ we can't use it 😞

@typeless
Copy link
Contributor

@bkcsoft Agreed. This would probably need to be fixed upstream.

@andreynering
Copy link
Contributor

I hope #2116 fixes the issue

@andreynering
Copy link
Contributor

Reopening, since it seems it not really fixed.

@lafriks lafriks modified the milestones: 1.6.0, 1.x.x Aug 12, 2018
@hoxnox
Copy link

hoxnox commented Aug 30, 2018

Same for me.

Caused by: hudson.plugins.git.GitException: Command "/usr/local/git/bin/git fetch --tags --progress ssh://sources.acme.com/repo.git +refs/heads/*:refs/remotes/origin/*" returned status code 128:
stdout: 
stderr: Gitea: Internal error
Failed to get repository: database is locked

@typeless
Copy link
Contributor

Potential alternatives that could fix this:

https://github.com/bvinc/go-sqlite-lite
https://github.com/crawshaw/sqlite

@bvinc
Copy link

bvinc commented Sep 1, 2018

Author of go-sqlite-lite here. My driver, and also crawshaw's driver, don't provide a database/sql interface, so they would be extremely inconvenient to use. But for a few reasons, it would probably help with locking errors. My driver gives complete control over connections, which means you'll only get locking errors if you're doing something directly to cause them. The crawshaw driver defaults to using shared cache mode and WAL mode, and uses the unlock_notify API to prevent locking issues with shared cache mode.

I think the current mattn driver is the best one to use if you require a database/sql driver. Their FAQ mentions setting the the maximum number of open connections to 1. Have you tried that?

https://github.com/mattn/go-sqlite3#faq

@lunny
Copy link
Member Author

lunny commented Sep 2, 2018

@bvinc thanks for your reply, It's a good idea to send a PR to set open connections to 1 when using mattn/go-sqlite3

@hoxnox
Copy link

hoxnox commented Sep 4, 2018

What is the best way to migrate from sqlite to postgresql?

@typeless
Copy link
Contributor

typeless commented Oct 21, 2018

Now that mattn/go-sqlite3#439 is merged, we can either use the tip right away or wait for the new release of go-sqlite3 to fix the issue. Note that to enable this functionality, we have to add a new build tag sqlite_unlock_notify.

Edit: I hope the users don't have to specify -tags sqlite sqlite_unlock_notify besides just -tags sqlite in that it would be better to not expose the nuances of a dependent package. I don't know how it should be done though.

Edit2: I changed my mind. If one wants to build Gitea from source, he should be capable of handling the additional flag. We should update the documentation accordingly though.

@lunny
Copy link
Member Author

lunny commented Oct 21, 2018

at first, we need a PR to update go-sqlite3 library.

@lunny
Copy link
Member Author

lunny commented Oct 23, 2018

Even #5144 resolved single process issue. There is some issues when pushing via SSH. This should be closed by #4886.

@lunny lunny reopened this Oct 23, 2018
@sapk
Copy link
Member

sapk commented Oct 23, 2018

@typeless go-sqlite3 library with sqlite_unlock_notify tag seems to not build on win 32bit https://drone.gitea.io/go-gitea/gitea/3536/18

@typeless
Copy link
Contributor

@sapk Similar to mattn/go-sqlite3#238. Will submit a fix soon.

@sapk
Copy link
Member

sapk commented Oct 23, 2018

@typeless exactly what I was searching but I use the terms uint too large and I found nothing ...

@typeless
Copy link
Contributor

I have submitted mattn/go-sqlite3#654.
However, I cannot find a computer with Windows/386 to test. 😨

@sapk
Copy link
Member

sapk commented Oct 23, 2018

@typeless use gox to cross-compile ?

@typeless
Copy link
Contributor

typeless commented Oct 24, 2018

@sapk I just installed it. But after typing gox -osarch="windows/linux" -tags sqlite_unlock_notify
, it only prints Number of parallel builds: 31. I've not figured out how to use it to run go test either.

Edit: fixed at mattn/go-sqlite3#655

@lunny
Copy link
Member Author

lunny commented Oct 24, 2018

Just type make release-windows on gitea?

@typeless
Copy link
Contributor

typeless commented Oct 24, 2018

#5162 fixes the CI failure and TAGS='bindata sqlite sqlite_unlock_notify' make release-windows passes.

@lunny
Copy link
Member Author

lunny commented Feb 26, 2019

Closed this since it's not a system problem any more. If there is any issue please fire another issue.

@lunny lunny closed this as completed Feb 26, 2019
@lunny lunny removed this from the 1.x.x milestone Feb 26, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
6543 pushed a commit to 6543-forks/gitea that referenced this issue Feb 26, 2024
During registration, one may be required to give their email address, to
be verified and activated later. However, if one makes a mistake, a
typo, they may end up with an account that cannot be activated due to
having a wrong email address.

They can still log in, but not change the email address, thus, no way to
activate it without help from an administrator.

To remedy this issue, lets allow changing the email address for logged
in, but not activated users.

This fixes gitea#17785.

Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
(cherry picked from commit aaaece28e4c6a8980cef932e224e84933d7c9262)
(cherry picked from commit 639dafabec0a5c1f943b44ca02f72c5ba2fc5e10)
(cherry picked from commit d699c12)

[GITEA] Allow changing the email address before activation (squash) cache is always active

This needs to be revisited because the MailResendLimit is not enforced
and turns out to not be tested.

See e7cb8da * Always enable caches (go-gitea#28527)

(cherry picked from commit 43ded8e)

Rate limit pre-activation email change separately

Changing the email address before any email address is activated should
be subject to a different rate limit than the normal activation email
resending. If there's only one rate limit for both, then if a newly
signed up quickly discovers they gave a wrong email address, they'd have
to wait three minutes to change it.

With the two separate limits, they don't - but they'll have to wait
three minutes before they can change the email address again.

The downside of this setup is that a malicious actor can alternate
between resending and changing the email address (to something like
`user+$idx@domain`, delivered to the same inbox) to effectively halving
the rate limit. I do not think there's a better solution, and this feels
like such a small attack surface that I'd deem it acceptable.

The way the code works after this change is that `ActivatePost` will now
check the `MailChangeLimit_user` key rather than `MailResendLimit_user`,
and if we're within the limit, it will set `MailChangedJustNow_user`. The
`Activate` method - which sends the activation email, whether it is a
normal resend, or one following an email change - will check
`MailChangedJustNow_user`, and if it is set, it will check the rate
limit against `MailChangedLimit_user`, otherwise against
`MailResendLimit_user`, and then will delete the
`MailChangedJustNow_user` key from the cache.

Fixes go-gitea#2040.

Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
(cherry picked from commit e35d2af2e56f4ecb3a4f6d1109d02c8aa1a6d182)
(cherry picked from commit 03989418a70d3445e0edada7fbe5a4151d7836b1)
(cherry picked from commit f50e0dfe5e90d6a31c5b59e687580e8b2725c22b)
(cherry picked from commit cad9184a3653e6c80de2e006a0d699b816980987)
(cherry picked from commit e2da5d7fe13a685606913a131687a94f9f5fcfeb)
(cherry picked from commit 3a80534d4db523efe56b368489f81dc1cb2c99f7)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug
Projects
None yet
10 participants