From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Thu, 15 Feb 2024 14:16:50 +0000 (+0100)
Subject: lmdb: remove mapasync mode, it was always a lie
X-Git-Tag: auth-4.9.0-beta2^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=092dcb659feafa750a28f1e935fa5bf5c5d5199b;p=thirdparty%2Fpdns.git

lmdb: remove mapasync mode, it was always a lie

fixes #12888
---

diff --git a/docs/backends/lmdb.rst b/docs/backends/lmdb.rst
index 178dcf738c..db573f447b 100644
--- a/docs/backends/lmdb.rst
+++ b/docs/backends/lmdb.rst
@@ -51,10 +51,14 @@ Default is 2 on 32 bits systems, and 64 on 64 bits systems.
 ``lmdb-sync-mode``
 ^^^^^^^^^^^^^^^^^^
 
-* Synchronisation mode: sync, nosync, nometasync, mapasync
-* Default: mapasync
+  .. versionchanged:: 4.9.0
 
-``sync``
+  ``mapasync`` choice removed
+
+* Synchronisation mode: sync, nosync, nometasync
+* Default: sync
+
+``sync`` (default since 4.9.0)
   LMDB synchronous mode. Safest option, but also slightly slower. Can  also be enabled with ``lmdb-sync-mode=``
 
 ``nosync``
@@ -64,8 +68,9 @@ Default is 2 on 32 bits systems, and 64 on 64 bits systems.
 ``nometasync``
   flush system buffers to disk only once per transaction, omit the metadata flush. This maintains database integrity, but can potentially lose the last committed transaction if the operating system crashes.
 
-``mapasync`` (default)
-  Use asynchronous flushes to disk. As with nosync, a system crash can then corrupt the database or lose the last transactions.
+``mapasync`` (default before 4.9.0)
+  Due to a bug before version 4.9.0, this actually gave ``sync`` behaviour.
+  The ``mapasync`` choice has been removed in version 4.9.0.
 
 .. _setting-lmdb-schema-version:
 
diff --git a/docs/upgrading.rst b/docs/upgrading.rst
index 1fce623997..e43e0f593b 100644
--- a/docs/upgrading.rst
+++ b/docs/upgrading.rst
@@ -23,6 +23,10 @@ Various settings, deprecated since 4.5.0, have been removed.
 * :ref:`setting-slave` is now :ref:`setting-secondary`
 * :ref:`setting-superslave` is now :ref:`setting-autosecondary`
 
+In :ref:`setting-lmdb-sync-mode`, the previous default ``mapasync`` is no longer a valid value.
+Due to a bug, it was interpreted as ``sync`` in previous versions.
+To avoid operational surprises, ``sync`` is the new default value.
+
 Renamed options
 ^^^^^^^^^^^^^^^
 
diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc
index e528d63295..6038d989e0 100644
--- a/modules/lmdbbackend/lmdbbackend.cc
+++ b/modules/lmdbbackend/lmdbbackend.cc
@@ -662,8 +662,6 @@ LMDBBackend::LMDBBackend(const std::string& suffix)
     d_asyncFlag = MDB_NOSYNC;
   else if (syncMode == "nometasync")
     d_asyncFlag = MDB_NOMETASYNC;
-  else if (syncMode == "mapasync")
-    d_asyncFlag = MDB_MAPASYNC;
   else if (syncMode.empty() || syncMode == "sync")
     d_asyncFlag = 0;
   else
@@ -2772,7 +2770,7 @@ public:
   void declareArguments(const string& suffix = "") override
   {
     declare(suffix, "filename", "Filename for lmdb", "./pdns.lmdb");
-    declare(suffix, "sync-mode", "Synchronisation mode: nosync, nometasync, mapasync, sync", "mapasync");
+    declare(suffix, "sync-mode", "Synchronisation mode: nosync, nometasync, sync", "sync");
     // there just is no room for more on 32 bit
     declare(suffix, "shards", "Records database will be split into this number of shards", (sizeof(void*) == 4) ? "2" : "64");
     declare(suffix, "schema-version", "Maximum allowed schema version to run on this DB. If a lower version is found, auto update is performed", std::to_string(SCHEMAVERSION));