Add setting SQLITE_JOURNAL_MODE to enable WAL

[noerw]

WAL journal mode is supported in SQLite and provides much better concurrent write performance than the default mode. This likely enables even large instances to get along with a sqlite DB (though I didn't do any load testing yet — does someone have a load testing setup ready?).

After empirically confirming WAL performs better, it could be made the default setting, but installations with builds for non-unix & -windows systems may break (see disclaimer in the linked sqlite doc). Not sure if thats a real-world issue we need to consider, as gitea only provides builds for unix & windows afaik.

[zeripath]

Tbh I'd be happy if we just changed it to wal by default or even provided an os detection that made the decision to use wal by default based on os

[noerw]

@zeripath Sounds reasonable. Would you prefer choosing the appropriate mode during runtime or build time?

[noerw]

• I made it select the mode via a runtime OS check. Would be nice if someone could check if WAL works on the BSDs.

• I also removed the setting, as the automatic selection should be good enough. If you think the setting is still needed, feel free to revert 1f744f9

• The default journal mode DELETE could be changed to TRUNCATE, which provides better performance and has no drawbacks apart from the journal file lingering around:

  On many systems, truncating a file is much faster than deleting the file since the containing directory does not need to be changed.

[wxiaoguang]

I'd like to trust the SQLite document and think that SQLite WAL works on all modern OS's native filesystem.

However, according to the document, SQLite's Shared-Memory WAL doesn't work for filesystems without shared-memory support.

There is a section for non-Shared-Memory WAL: "8. Use of WAL Without Shared-Memory"

So IMO the problem is not related to OS, but related to filesystem and how the program uses SQLite.

[zeripath]

Damn, sorry I was too hasty and misremembered what SQLite says - I should have rechecked the docs.

We should restore the option. (and likely back to your original commit - see below.)

---

Now, what to do about the default/blank setting of that option.

Unfortunately, I guess we can't set it to WAL because that could actually be a breaking change and I would not be surprised if it would break some people's installs. Docker can use "network filesystems" internally (especially docker for windows) so they're already banned from WAL.

We could do the soft-default option - add an option, default it to blank, but at install time provide it as an option with a default set to WAL. But the problem is how to detect if it is failing, if SQLite just barfs at open time that's a simple test. (It would of course be best if it just fell back to DELETE.) If it doesn't barf on open, I worry it could just quietly corrupt itself or barf only on the first write which could lead to some very confusing issues. (you can check if sqlite has quietly fallen back to DELETE by opening the db with sqlite3 and running PRAGMA schema_mode;)

Of course if SQLite just auto falls back to DELETE when it detects that WAL would not work then all of my worries are irrelevant...

I guess the simplest thing to do is to just add the option but leave it blank.

Otherwise we'll need to test common configurations and see if we can get things to barf. If I had to guess at a likely failure configuration it would be docker for windows.

PS @noerw runtime.GOOS is a constant so any code dependent on should be resolved at compile time.

[noerw]

I just did a quick test, running a build of this current PR with a PRAGMA journal_mode=DELETE; db on a cifs samba mount.
Sqlite happily enabled WAL mode there.
As this is not supported according to the sqlite docs, enabling WAL and falling back (explicitly or implicitly within sqlite) is not an option without elaborate detection of the underlying OS & filesystem.

So I think the way to go is to revert to the original version of this PR

[zeripath]

So thinking on I think we should not set the _journal_mode if the SQLITE_JOURNAL_MODE has not been explicitly provided.

This is because clever users might have already changed to use WAL in their dbs and you never know sqlite might change their default in future.

From eef3886d433f04fddc28d4d15c305e3f4db41ea5 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Sat, 30 Jul 2022 13:34:26 +0100
Subject: [PATCH] Do not set _journal_mode if `SQLITE_JOURNAL_MODE` is unset

Signed-off-by: Andrew Thornton <art27@cantab.net>

diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini
index 6dc0e728f..1c6a7e3b7 100644
--- a/custom/conf/app.example.ini
+++ b/custom/conf/app.example.ini
@@ -313,7 +313,7 @@ USER = root
 ;DB_TYPE = sqlite3
 ;PATH= ; defaults to data/gitea.db
 ;SQLITE_TIMEOUT = ; Query timeout defaults to: 500
-;SQLITE_JOURNAL_MODE = ; defaults to DELETE, can be used to enable WAL mode. https://www.sqlite.org/pragma.html#pragma_journal_mode
+;SQLITE_JOURNAL_MODE = ; defaults to sqlite database default (often DELETE), can be used to enable WAL mode. https://www.sqlite.org/pragma.html#pragma_journal_mode
 ;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md
index 9a1c40dd0..cb2b9526d 100644
--- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md
+++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md
@@ -382,7 +382,7 @@ The following configuration set `Content-Type: application/vnd.android.package-a
     - `verify-ca`: Enable TLS with verification of the database server certificate against its root certificate.
     - `verify-full`: Enable TLS and verify the database server name matches the given certificate in either the `Common Name` or `Subject Alternative Name` fields.
 - `SQLITE_TIMEOUT`: **500**: Query timeout for SQLite3 only.
-- `SQLITE_JOURNAL_MODE`: **DELETE**: Change journal mode for SQlite3. Can be used to enable [WAL mode](https://www.sqlite.org/wal.html) when high load causes write congestion. See [SQlite3 docs](https://www.sqlite.org/pragma.html#pragma_journal_mode) for possible values.
+- `SQLITE_JOURNAL_MODE`: **""**: Change journal mode for SQlite3. Can be used to enable [WAL mode](https://www.sqlite.org/wal.html) when high load causes write congestion. See [SQlite3 docs](https://www.sqlite.org/pragma.html#pragma_journal_mode) for possible values. Defaults to the default for the database file, often DELETE.
 - `ITERATE_BUFFER_SIZE`: **50**: Internal buffer size for iterating.
 - `CHARSET`: **utf8mb4**: For MySQL only, either "utf8" or "utf8mb4". NOTICE: for "utf8mb4" you must use MySQL InnoDB > 5.6. Gitea is unable to check this.
 - `PATH`: **data/gitea.db**: For SQLite3 only, the database file path.
diff --git a/modules/setting/database.go b/modules/setting/database.go
index 0b37826f2..6c241d4d2 100644
--- a/modules/setting/database.go
+++ b/modules/setting/database.go
@@ -92,7 +92,7 @@ func InitDBConfig() {
 
 	Database.Path = sec.Key("PATH").MustString(filepath.Join(AppDataPath, "gitea.db"))
 	Database.Timeout = sec.Key("SQLITE_TIMEOUT").MustInt(500)
-	Database.SQliteJournalMode = sec.Key("SQLITE_JOURNAL_MODE").MustString("DELETE")
+	Database.SQliteJournalMode = sec.Key("SQLITE_JOURNAL_MODE").MustString("")
 
 	Database.MaxIdleConns = sec.Key("MAX_IDLE_CONNS").MustInt(2)
 	if Database.UseMySQL {
@@ -139,8 +139,12 @@ func DBConnStr() (string, error) {
 		if err := os.MkdirAll(path.Dir(Database.Path), os.ModePerm); err != nil {
 			return "", fmt.Errorf("Failed to create directories: %v", err)
 		}
-		connStr = fmt.Sprintf("file:%s?cache=shared&mode=rwc&_busy_timeout=%d&_txlock=immediate&_journal_mode=%s",
-			Database.Path, Database.Timeout, Database.SQliteJournalMode)
+		journalMode := ""
+		if Database.SQliteJournalMode != "" {
+			journalMode = "&_journal_mode=" + Database.SQliteJournalMode
+		}
+		connStr = fmt.Sprintf("file:%s?cache=shared&mode=rwc&_busy_timeout=%d&_txlock=immediate%s",
+			Database.Path, Database.Timeout, journalMode)
 	default:
 		return "", fmt.Errorf("Unknown database type: %s", Database.Type)
 	}
-- 
2.37.1

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (main@692707f). Click here to learn what that means.
The diff coverage is 25.00%.

@@           Coverage Diff           @@
##             main   #20535   +/-   ##
=======================================
  Coverage        ?   46.96%           
=======================================
  Files           ?      978           
  Lines           ?   135497           
  Branches        ?        0           
=======================================
  Hits            ?    63633           
  Misses          ?    64058           
  Partials        ?     7806           

Impacted Files	Coverage Δ
modules/setting/database.go	51.90% <25.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

[6543]

.