From a06fe4f168ab72d5af31666988f1d5b7476e58b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9B=D0=B5=D0=BE=D0=BD=D0=B8=D0=B4=20=D0=AE=D1=80=D1=8C?= =?UTF-8?q?=D0=B5=D0=B2=20=28Leonid=20Yuriev=29?= Date: Sun, 25 Dec 2022 19:56:50 +0300 Subject: [PATCH] =?UTF-8?q?mdbx:=20=D0=BF=D0=B5=D1=80=D0=B5=D1=80=D0=B0?= =?UTF-8?q?=D0=B1=D0=BE=D1=82=D0=BA=D0=B0=20=D0=BA=D0=BE=D0=BD=D1=82=D1=80?= =?UTF-8?q?=D0=BE=D0=BB=D1=8F=20"=D0=BD=D0=B5=D0=BA=D0=BE=D0=B3=D0=B5?= =?UTF-8?q?=D1=80=D0=B5=D0=BD=D1=82=D0=BD=D0=BE=D1=81=D1=82=D0=B8"=20?= =?UTF-8?q?=D0=B4=D0=BB=D1=8F=20=D1=83=D0=BC=D0=B5=D0=BD=D1=8C=D1=88=D0=B5?= =?UTF-8?q?=D0=BD=D0=B8=D1=8F=20=D0=BD=D0=B0=D0=BA=D0=BB=D0=B0=D0=B4=D0=BD?= =?UTF-8?q?=D1=8B=D1=85=20=D1=80=D0=B0=D1=81=D1=85=D0=BE=D0=B4=D0=BE=D0=B2?= =?UTF-8?q?.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Существует проблема https://libmdbx.dqdkfa.ru/dead-github/issues/269, которая проявляется только при специфической неупорядоченности внутри ядра ОС, когда страницы, записанные в файл отображенный в память, становятся видны в памяти посредством работы unified page cache: - если записанная последней мета-страница "обгоняет" ранее записанные, т.е. когда записанное в файл позже становится видимым в отображении раньше, чем записанное ранее. Теперь, вместо постоянной полной сверки записываемых страниц, выполняется легковесная проверка при старте транзакций, с переключением в режим "как раньше" при обнаружении проблемы. В результате, в некоторых сценариях возвращается 5-10% производительности, а в отдельных синтетических тестах до 30%. --- src/core.c | 82 +++++++++++++++++++++++++++++++++++++++++++++---- src/internals.h | 5 +++ 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/src/core.c b/src/core.c index dc530f7d..26fd9c08 100644 --- a/src/core.c +++ b/src/core.c @@ -4546,10 +4546,15 @@ typedef struct iov_ctx { __must_check_result static int iov_init(MDBX_txn *const txn, iov_ctx_t *ctx, size_t items, size_t npages, - mdbx_filehandle_t fd) { + mdbx_filehandle_t fd, + bool check_coherence) { ctx->env = txn->mt_env; ctx->ior = &txn->mt_env->me_ioring; ctx->fd = fd; + ctx->coherency_timestamp = + (check_coherence || txn->mt_env->me_lck->mti_pgop_stat.incoherence.weak) + ? 0 + : UINT64_MAX /* не выполнять сверку */; ctx->err = osal_ioring_prepare(ctx->ior, items, pgno_align2os_bytes(txn->mt_env, npages)); if (likely(ctx->err == MDBX_SUCCESS)) { @@ -4582,9 +4587,63 @@ static void iov_callback4dirtypages(iov_ctx_t *ctx, size_t offset, void *data, MDBX_ASAN_UNPOISON_MEMORY_REGION(rp, bytes); osal_flush_incoherent_mmap(rp, bytes, env->me_os_psize); /* check with timeout as the workaround - * for https://libmdbx.dqdkfa.ru/dead-github/issues/269 */ - if (unlikely(memcmp(wp, rp, bytes))) { + * for https://libmdbx.dqdkfa.ru/dead-github/issues/269 + * + * Проблема проявляется только при неупорядоченности: если записанная + * последней мета-страница "обгоняет" ранее записанные, т.е. когда + * записанное в файл позже становится видимым в отображении раньше, + * чем записанное ранее. + * + * Исходно здесь всегда выполнялась полная сверка. Это давало полную + * гарантию защиты от проявления проблемы, но порождало накладные расходы. + * В некоторых сценариях наблюдалось снижение производительности до 10-15%, + * а в синтетических тестах до 30%. Конечно никто не вникал в причины, + * а просто останавливался на мнении "libmdbx не быстрее LMDB", + * например: https://clck.ru/3386er + * + * Поэтому после серии экспериментов и тестов реализовано следующее: + * 0. Посредством опции сборки MDBX_FORCE_CHECK_MMAP_COHERENCY=1 + * можно включить полную сверку после записи. + * Остальные пункты являются взвешенным компромиссом между полной + * гарантией обнаружения проблемы и бесполезными затратами на системах + * без этого недостатка. + * 1. При старте транзакций проверяется соответствие выбранной мета-страницы + * корневым страницам b-tree проверяется. Эта проверка показала себя + * достаточной без сверки после записи. При обнаружении "некогерентности" + * эти случаи подсчитываются, а при их ненулевом счетчике выполняется + * полная сверка. Таким образом, произойдет переключение в режим полной + * сверки, если показавшая себя достаточной проверка заметит проявление + * проблемы хоты-бы раз. + * 2. Сверка не выполняется при фиксации транзакции, так как: + * - при наличии проблемы "не-когерентности" (при отложенном копировании + * или обновлении PTE, после возврата из write-syscall), проверка + * в этом процессе не гарантирует актуальность данных в другом + * процессе, который может запустить транзакцию сразу после коммита; + * - сверка только последнего блока позволяет почти восстановить + * производительность в больших транзакциях, но одновременно размывает + * уверенность в отсутствии сбоев, чем обесценивает всю затею; + * - после записи данных будет записана мета-страница, соответствие + * которой корневым страницам b-tree проверяется при старте + * транзакций, и только эта проверка показала себя достаточной; + * 3. При спиллинге производится полная сверка записанных страниц. Тут был + * соблазн сверять не полностью, а например начало и конец каждого блока. + * Но при спиллинге возможна ситуация повторного вытеснения страниц, в + * том числе large/overflow. При этом возникает риск прочитать в текущей + * транзакции старую версию страницы, до повторной записи. В этом случае + * могут возникать крайне редкие невоспроизводимые ошибки. С учетом того + * что спиллинг выполняет крайне редко, решено отказаться от экономии + * в пользу надежности. */ +#ifndef MDBX_FORCE_CHECK_MMAP_COHERENCY +#define MDBX_FORCE_CHECK_MMAP_COHERENCY 0 +#endif /* MDBX_FORCE_CHECK_MMAP_COHERENCY */ + if ((MDBX_FORCE_CHECK_MMAP_COHERENCY || + ctx->coherency_timestamp != UINT64_MAX) && + unlikely(memcmp(wp, rp, bytes))) { ctx->coherency_timestamp = 0; + env->me_lck->mti_pgop_stat.incoherence.weak = + (env->me_lck->mti_pgop_stat.incoherence.weak >= INT32_MAX) + ? INT32_MAX + : env->me_lck->mti_pgop_stat.incoherence.weak + 1; WARNING("catch delayed/non-arrived page %" PRIaPGNO " %s", wp->mp_pgno, "(workaround for incoherent flaw of unified page/buffer cache)"); do @@ -5074,7 +5133,8 @@ __cold static int txn_spill_slowpath(MDBX_txn *const txn, MDBX_cursor *const m0, #if defined(_WIN32) || defined(_WIN64) txn->mt_env->me_overlapped_fd ? txn->mt_env->me_overlapped_fd : #endif - txn->mt_env->me_lazy_fd); + txn->mt_env->me_lazy_fd, + true); if (unlikely(rc != MDBX_SUCCESS)) goto bailout; @@ -8530,6 +8590,11 @@ static bool coherency_check(const MDBX_env *env, const txnid_t txnid, ok = false; } } + if (unlikely(!ok) && report) + env->me_lck->mti_pgop_stat.incoherence.weak = + (env->me_lck->mti_pgop_stat.incoherence.weak >= INT32_MAX) + ? INT32_MAX + : env->me_lck->mti_pgop_stat.incoherence.weak + 1; return ok; } @@ -8579,11 +8644,16 @@ static int coherency_check_written(const MDBX_env *env, const txnid_t txnid, const bool report = !(timestamp && *timestamp); const txnid_t head_txnid = meta_txnid(meta); if (unlikely(head_txnid < MIN_TXNID || (head_txnid < txnid))) { - if (report) + if (report) { + env->me_lck->mti_pgop_stat.incoherence.weak = + (env->me_lck->mti_pgop_stat.incoherence.weak >= INT32_MAX) + ? INT32_MAX + : env->me_lck->mti_pgop_stat.incoherence.weak + 1; WARNING("catch %s txnid %" PRIaTXN " for meta_%" PRIaPGNO " %s", (head_txnid < MIN_TXNID) ? "invalid" : "unexpected", head_txnid, bytes2pgno(env, ptr_dist(meta, env->me_map)), "(workaround for incoherent flaw of unified page/buffer cache)"); + } return coherency_timeout(timestamp, 0); } return coherency_check_readed(env, head_txnid, meta->mm_dbs, meta, timestamp); @@ -11678,7 +11748,7 @@ int mdbx_txn_commit_ex(MDBX_txn *txn, MDBX_commit_latency *latency) { iov_ctx_t write_ctx; rc = iov_init(txn, &write_ctx, txn->tw.dirtylist->length, - txn->tw.dirtylist->pages_including_loose, fd); + txn->tw.dirtylist->pages_including_loose, fd, false); if (unlikely(rc != MDBX_SUCCESS)) { ERROR("txn-%s: error %d", "iov-init", rc); goto fail; diff --git a/src/internals.h b/src/internals.h index eb14a15b..684628e5 100644 --- a/src/internals.h +++ b/src/internals.h @@ -622,6 +622,11 @@ typedef struct pgop_stat { MDBX_atomic_uint64_t prefault; /* Number of prefault write operations */ MDBX_atomic_uint64_t mincore; /* Number of mincore() calls */ + MDBX_atomic_uint32_t + incoherence; /* number of https://libmdbx.dqdkfa.ru/dead-github/issues/269 + caught */ + MDBX_atomic_uint32_t reserved; + /* Статистика для профилирования GC. * Логически эти данные может быть стоит вынести в другую структуру, * но разница будет сугубо косметическая. */