14.0.4: PostgreSQL Duplicate key value violates unique constraint lock_key_index

[torrentkino]

Steps to reproduce

1. Use Nextcloud-14.0.4 + php-fpm + nginx + PostgreSQL
2. Only one single user needed

Expected behaviour

No error messages.

Actual behaviour

Too many error messages:

$ sudo egrep '2018-11-29.*duplicate key value violates unique constraint "lock_key_index"'
/var/log/postgresql/postgresql-11-main.log | wc -l
15484

Those error messages look like this:
nextcloud@nextcloud ERROR: duplicate key value violates unique constraint "lock_key_index"
nextcloud@nextcloud DETAIL: Key (key)=(files/5f46d7639d8049fc652b79286a38d339) already exists.
nextcloud@nextcloud STATEMENT: INSERT INTO "oc_file_locks" ("key", "lock", "ttl") VALUES($1, $2, $3)

Server configuration

Operating system: Debian/Buster

Web server: nginx-1.14.1-1

Database: PostgreSQL-11

PHP version: 7.3

Nextcloud version: 14.0.4

Updated from an older Nextcloud/ownCloud or fresh install: Updates vom 13.x

Where did you install Nextcloud from: Official package. No docker.

List of activated apps:

Enabled:

• accessibility: 1.0.1
• activity: 2.7.0
• audioplayer: 2.4.1
• bookmarks: 0.14.2
• bruteforcesettings: 1.2.0
• calendar: 1.6.4
• cloud_federation_api: 0.0.1
• comments: 1.4.0
• contacts: 2.1.7
• dav: 1.6.0
• deck: 0.5.0
• federatedfilesharing: 1.4.0
• federation: 1.4.0
• files: 1.9.0
• files_pdfviewer: 1.3.2
• files_sharing: 1.6.2
• files_texteditor: 2.6.0
• files_trashbin: 1.4.1
• files_versions: 1.7.1
• files_videoplayer: 1.3.0
• firstrunwizard: 2.3.0
• gallery: 18.1.0
• logreader: 2.0.0
• lookup_server_connector: 1.2.0
• news: 13.0.3
• nextcloud_announcements: 1.3.0
• notes: 2.5.0
• notifications: 2.2.1
• oauth2: 1.2.1
• password_policy: 1.4.0
• previewgenerator: 2.0.0
• provisioning_api: 1.4.0
• serverinfo: 1.4.0
• sharebymail: 1.4.0
• support: 1.0.0
• survey_client: 1.2.0
• systemtags: 1.4.0
• theming: 1.5.0
• twofactor_backupcodes: 1.3.1
• updatenotification: 1.4.1
• workflowengine: 1.4.0
  Disabled:
• admin_audit
• encryption
• files_external
• user_external
• user_ldap

Nextcloud configuration:

{
"system": {
"instanceid": "REMOVED SENSITIVE VALUE",
"passwordsalt": "REMOVED SENSITIVE VALUE",
"secret": "REMOVED SENSITIVE VALUE",
"trusted_domains": [
"foo.torrentkino.de"
],
"datadirectory": "REMOVED SENSITIVE VALUE",
"overwrite.cli.url": "https://foo.torrentkino.de",
"memcache.local": "\OC\Memcache\APCu",
"dbtype": "pgsql",
"version": "14.0.4.2",
"dbname": "REMOVED SENSITIVE VALUE",
"dbhost": "REMOVED SENSITIVE VALUE",
"dbport": "",
"dbtableprefix": "oc_",
"dbuser": "REMOVED SENSITIVE VALUE",
"dbpassword": "REMOVED SENSITIVE VALUE",
"installed": true,
"maintenance": false,
"theme": "",
"loglevel": 2,
"mail_smtpmode": "smtp",
"mail_smtpauthtype": "LOGIN",
"mail_smtpsecure": "tls",
"mail_from_address": "REMOVED SENSITIVE VALUE",
"mail_domain": "REMOVED SENSITIVE VALUE",
"mail_smtpauth": 1,
"mail_smtphost": "REMOVED SENSITIVE VALUE",
"mail_smtpport": "587",
"mail_smtpname": "REMOVED SENSITIVE VALUE",
"mail_smtppassword": "REMOVED SENSITIVE VALUE",
"updater.release.channel": "stable"
}
}

Are you using external storage, if yes which one: No

Are you using encryption: No

Are you using an external user-backend, if yes which one: No

[nextcloud-bot]

GitMate.io thinks possibly related issues are #12586 (14.0.4), #366 (PostgreSQL: violation of constraints), #6343 (PostgreSQL : duplicate key value violates unique constraint error message), #12204 (Resolve ERROR: duplicate key value violates unique constraint "oc_credentials_pkey"), and #11779 (SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry * for key 'lock_key_index').

[MorrisJobke]

Caused by #12413 and it is on purpose:

We actually changed how the detection works, because there was a race condition in the old approach leading to real deadlocks and broken locks. Thus we changed the way from "INSERT INTO ... WHEN (SUBSELECT TO CHECK IF THIS EXISTS)". The old way had the problem that when a concurrent request also inserted right between the end of the subselect and the actual insert then the error was happening. So we now changed to trigger an insert and check if an exception is thrown. In this way we ignore it (because the code had the logic "only insert if it does not exist yet").

Long story short: if there is no such message in your nextcloud log then you can ignore the error. We tried to not cause this but unfortunately there was no clean way to solve this on all supported platforms except like this.

[dashdsrdash]

Nearly a thousand of these each day.

Postgres supports INSERT... ON CONFLICT [DO NOTHING | DO UPDATE] starting in 9.5. Current release is 11. Debian stable has 9.6. I think you can require 9.5 or later by now.

[MorrisJobke]

Nearly a thousand of these each day.

Postgres supports INSERT... ON CONFLICT [DO NOTHING | DO UPDATE] starting in 9.5. Current release is 11. Debian stable has 9.6. I think you can require 9.5 or later by now.

Unfortunately we also support SQLite, MySQL, MariaDB and Oracle as well. Then the combined SQL syntax is not that impressive. 😢

[dashdsrdash]

So obviously you have a translation layer that takes a high-level "let me do an INSERT" and turns it into the right variant for each of your supported databases. That's where this goes.

If you're writing SQL by hand and hoping that it works on all of those DBs... well, this is a case where it isn't working.

[MorrisJobke]

So obviously you have a translation layer that takes a high-level "let me do an INSERT" and turns it into the right variant for each of your supported databases. That's where this goes.

If you're writing SQL by hand and hoping that it works on all of those DBs... well, this is a case where it isn't working.

We are happy to get PRs that head into this direction obviously. For the correct location have a look at the linked PR.

We can also give you further guidance on this topic if you need help.

[dashdsrdash]

Sadly, I am not proficient at PHP. It looks like
AdapterPgSql.php needs the addition of something like
public function insertIfNotExist (very much like what you put in the general adapter)
with the addition of ON CONFLICT ( $input[$key] ) DO NOTHING; at the end of the INSERT INTO phrase, then you can drop the SELECT check because Postgres will take care of it for you.

Sqlite also implements ON CONFLICT (value) DO NOTHING, as of 3.24.0. Debian stable (which I'm using as a benchmark for "can we expect a year-old distro to have this") does not use a sufficiently new sqlite, but it is available in backports (3.25.3-2~bpo9+1).

Meanwhile, over in MySQL, changing the INSERT to INSERT IGNORE should do the same thing; that's been supported for at least 8 years.

The existing logic appears to be the best possible in oracle, although MERGE is suitable if you had a whole temporary table you wanted to bring in -- not the case here.

These changes will require updating documentation to require minimum versions of
Postgresql: 9.5 (rather than the currently specified 9)
sqlite3: 3.24.0 (currently unspecified)
and no change for MySQL/MariaDB or Oracle.

I hope that's helpful.

[oole]

@MorrisJobke I would like to contribute. And I think this issue would be a good place to start.

To me it seems like @dashdsrdash's proposal is quite reasonable.

I moved the query from

server/lib/private/Lock/DBLockingProvider.php

Line 135 in cff89c2

protected function initLockField(string $path, int $lock = 0): int {

to the Adapter.php, as a "one query fits all". And overrode it in the AdapterPgSql.php to have a PostgreSQL specific solution. (Of course this could be overridden by the other Adapters as well)

@MorrisJobke I would be happy to hear some feedback, whether this coincides with what you had in mind?

[MorrisJobke]

to the Adapter.php, as a "one query fits all". And overrode it in the AdapterPgSql.php to have a PostgreSQL specific solution. (Of course this could be overridden by the other Adapters as well)

That makes most sense and maybe helps with the problems of log spam that was reported by Postgres users. Do you have the code already available and want to send a pull request? Because then we can check it better.

[oole]

I created a PR here. I got a little distracted last week, so that took a bit longer.

I would be happy to get some feedback!

[jkuester]

Just wanted to leave this out here in case anyone is facing similar issues with a docker-compose setup. It was not immediately obvious to me how to prevent these messages from being logged (while waiting for an actual fix). If you are already using a custom postgresql.conf file, then you can just set the config there, but if you are just running with the default config, the easiest way to prevent logging all of these errors is just to add this line to the docker-compose.yml config for your postgresql container:
command: postgres -c log_min_messages=LOG

This will, of course, prevent other actual errors from being logged, but it is better than filling up the logs with these errors. See this page for more info on Postgresql's logging settings: https://www.postgresql.org/docs/9.1/runtime-config-logging.html