summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLuke Chen <luke.chen@mongodb.com>2022-12-05 16:26:11 +1100
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2022-12-05 05:54:13 +0000
commit5bb5d40c883c288d911ebff413d59ba8290c14fb (patch)
tree41a673e04ad302658927878ba1876bb6cbe22383
parenta98ea6e3be4a0c0aa93f691bbc5b4ae26f3dfafa (diff)
downloadmongo-5bb5d40c883c288d911ebff413d59ba8290c14fb.tar.gz
Import wiredtiger: 5784e4cb45f485304cc555981883c9999d691d7c from branch mongodb-5.0
ref: adba8ce291..5784e4cb45 for: 5.0.15 WT-9751 Fix memory leak if we fail eviction in single page rewrite
-rw-r--r--src/third_party/wiredtiger/import.data2
-rw-r--r--src/third_party/wiredtiger/src/btree/bt_discard.c15
-rw-r--r--src/third_party/wiredtiger/src/btree/bt_split.c9
-rw-r--r--src/third_party/wiredtiger/src/evict/evict_page.c147
-rw-r--r--src/third_party/wiredtiger/test/format/failure_configs/CONFIG.WT-9751138
5 files changed, 245 insertions, 66 deletions
diff --git a/src/third_party/wiredtiger/import.data b/src/third_party/wiredtiger/import.data
index 7f18ae7d4ff..e9d721d6575 100644
--- a/src/third_party/wiredtiger/import.data
+++ b/src/third_party/wiredtiger/import.data
@@ -2,5 +2,5 @@
"vendor": "wiredtiger",
"github": "wiredtiger/wiredtiger.git",
"branch": "mongodb-5.0",
- "commit": "adba8ce2917cd4d131237b7ccf4fc89199e26480"
+ "commit": "5784e4cb45f485304cc555981883c9999d691d7c"
}
diff --git a/src/third_party/wiredtiger/src/btree/bt_discard.c b/src/third_party/wiredtiger/src/btree/bt_discard.c
index 2e8e333f9e6..6d6c7b536fe 100644
--- a/src/third_party/wiredtiger/src/btree/bt_discard.c
+++ b/src/third_party/wiredtiger/src/btree/bt_discard.c
@@ -163,6 +163,13 @@ __free_page_modify(WT_SESSION_IMPL *session, WT_PAGE *page)
break;
}
__wt_free(session, multi->supd);
+ /*
+ * Discard the new disk images if they are not NULL. If the new disk images are NULL,
+ * they must have been instantiated into memory. Otherwise, we have a failure in
+ * eviction after reconciliation. If the split code only successfully instantiates a
+ * subset of new pages into memory, free the instantiated pages and the new disk images
+ * of the pages not in memory. We will redo reconciliation next time we visit this page.
+ */
__wt_free(session, multi->disk_image);
__wt_free(session, multi->addr.addr);
}
@@ -172,8 +179,16 @@ __free_page_modify(WT_SESSION_IMPL *session, WT_PAGE *page)
/*
* Discard any replacement address: this memory is usually moved into the parent's WT_REF,
* but at the root that can't happen.
+ *
+ * Discard the new disk image if it is not NULL. If the new disk image is NULL, it must have
+ * been instantiated into memory. Otherwise, we have a failure in eviction after
+ * reconciliation and later we decide to discard the old disk image without loading the new
+ * disk image into memory. Free the new disk image in this case. If a checkpoint visits this
+ * page, it would write the new disk image even it hasn't been instantiated into memory.
+ * Therefore, no need to reconcile the page again if it remains clean.
*/
__wt_free(session, mod->mod_replace.addr);
+ __wt_free(session, mod->mod_disk_image);
break;
}
diff --git a/src/third_party/wiredtiger/src/btree/bt_split.c b/src/third_party/wiredtiger/src/btree/bt_split.c
index baa3a36456d..4de8e448350 100644
--- a/src/third_party/wiredtiger/src/btree/bt_split.c
+++ b/src/third_party/wiredtiger/src/btree/bt_split.c
@@ -2129,8 +2129,8 @@ __split_multi(WT_SESSION_IMPL *session, WT_REF *ref, bool closing)
__split_multi_inmem_final(session, page, &mod->mod_multi[i]);
/*
- * Pages with unresolved changes are not marked clean in reconciliation, do it now, then discard
- * the page.
+ * Page with changes not written in this reconciliation is not marked as clean, do it now, then
+ * discard the page.
*/
__wt_page_modify_clear(session, page);
__wt_page_out(session, &page);
@@ -2139,6 +2139,11 @@ __split_multi(WT_SESSION_IMPL *session, WT_REF *ref, bool closing)
err:
for (i = 0; i < new_entries; ++i)
__split_multi_inmem_fail(session, page, &mod->mod_multi[i], ref_new[i]);
+ /*
+ * Mark the page dirty to ensure it is reconciled again as we free the split disk images if
+ * we fail to instantiate any of them into memory.
+ */
+ __wt_page_modify_set(session, page);
}
__wt_free(session, ref_new);
diff --git a/src/third_party/wiredtiger/src/evict/evict_page.c b/src/third_party/wiredtiger/src/evict/evict_page.c
index 62c8c53ea4f..3cd661a9d7f 100644
--- a/src/third_party/wiredtiger/src/evict/evict_page.c
+++ b/src/third_party/wiredtiger/src/evict/evict_page.c
@@ -10,6 +10,7 @@
static int __evict_page_clean_update(WT_SESSION_IMPL *, WT_REF *, uint32_t);
static int __evict_page_dirty_update(WT_SESSION_IMPL *, WT_REF *, uint32_t);
+static int __evict_reconcile(WT_SESSION_IMPL *, WT_REF *, uint32_t);
static int __evict_review(WT_SESSION_IMPL *, WT_REF *, uint32_t, bool *);
/*
@@ -159,11 +160,27 @@ __wt_evict(WT_SESSION_IMPL *session, WT_REF *ref, uint8_t previous_state, uint32
WT_ERR(__evict_review(session, ref, flags, &inmem_split));
/*
- * If there was an in-memory split, the tree has been left in the state we want: there is
- * nothing more to do.
+ * If we decide to do an in-memory split. Do it now. If an in-memory split completes, the page
+ * stays in memory and the tree is left in the desired state: avoid the usual cleanup.
*/
- if (inmem_split)
+ if (inmem_split) {
+ WT_ERR(__wt_split_insert(session, ref));
goto done;
+ }
+
+ /* No need to reconcile the page if it is from a dead tree or it is clean. */
+ if (!tree_dead && __wt_page_is_modified(page))
+ WT_ERR(__evict_reconcile(session, ref, flags));
+
+ /*
+ * Fail 0.1% of the time after we have done reconciliation. We should always evict the page of a
+ * dead tree.
+ */
+ if (!closing && !tree_dead &&
+ __wt_failpoint(session, WT_TIMING_STRESS_FAILPOINT_EVICTION_FAIL_AFTER_RECONCILIATION, 10)) {
+ ret = EBUSY;
+ goto err;
+ }
/* Check we are not evicting an accessible internal page with an active split generation. */
WT_ASSERT(session,
@@ -330,6 +347,7 @@ __evict_page_dirty_update(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_
WT_MULTI multi;
WT_PAGE_MODIFY *mod;
bool closing;
+ void *tmp;
mod = ref->page->modify;
closing = FLD_ISSET(evict_flags, WT_EVICT_CALL_CLOSING);
@@ -392,13 +410,20 @@ __evict_page_dirty_update(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_
__wt_ref_out(session, ref);
WT_REF_SET_STATE(ref, WT_REF_DISK);
} else {
- /*
- * The split code works with WT_MULTI structures, build one for the disk image.
- */
+ /* The split code works with WT_MULTI structures, build one for the disk image. */
memset(&multi, 0, sizeof(multi));
multi.disk_image = mod->mod_disk_image;
-
- WT_RET(__wt_split_rewrite(session, ref, &multi));
+ /*
+ * Store the disk image to a temporary pointer in case we fail to rewrite the page and
+ * we need to link the new disk image back to the old disk image.
+ */
+ tmp = mod->mod_disk_image;
+ mod->mod_disk_image = NULL;
+ ret = __wt_split_rewrite(session, ref, &multi);
+ if (ret != 0) {
+ mod->mod_disk_image = tmp;
+ return (ret);
+ }
}
break;
@@ -487,29 +512,23 @@ __evict_child_check(WT_SESSION_IMPL *session, WT_REF *parent)
/*
* __evict_review --
- * Get exclusive access to the page and review the page and its subtree for conditions that
- * would block its eviction.
+ * Review the page and its subtree for conditions that would block its eviction.
*/
static int
__evict_review(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_flags, bool *inmem_splitp)
{
WT_BTREE *btree;
- WT_CACHE *cache;
WT_CONNECTION_IMPL *conn;
WT_DECL_RET;
WT_PAGE *page;
- uint32_t flags;
- bool closing, modified, snapshot_acquired;
- bool is_eviction_thread, use_snapshot_for_app_thread;
+ bool closing, modified;
*inmem_splitp = false;
btree = S2BT(session);
conn = S2C(session);
page = ref->page;
- flags = WT_REC_EVICT;
closing = FLD_ISSET(evict_flags, WT_EVICT_CALL_CLOSING);
- snapshot_acquired = false;
/*
* Fail if an internal has active children, the children must be evicted first. The test is
@@ -553,13 +572,9 @@ __evict_review(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_flags, bool
if (!__wt_page_can_evict(session, ref, inmem_splitp))
return (__wt_set_return(session, EBUSY));
- /*
- * Check for an append-only workload needing an in-memory split; we can't do this earlier
- * because in-memory splits require exclusive access. If an in-memory split completes, the
- * page stays in memory and the tree is left in the desired state: avoid the usual cleanup.
- */
+ /* Check for an append-only workload needing an in-memory split. */
if (*inmem_splitp)
- return (__wt_split_insert(session, ref));
+ return (0);
}
/* If the page is clean, we're done and we can evict. */
@@ -568,12 +583,12 @@ __evict_review(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_flags, bool
/*
* If we are trying to evict a dirty page that does not belong to history store(HS) and
- * checkpoint is processing the HS file, then avoid evicting the dirty non-HS page for now if
- * the cache is already dominated by dirty HS content.
+ * checkpoint is processing the HS file, avoid evicting the dirty non-HS page for now if the
+ * cache is already dominated by dirty HS content.
*
- * Evicting a non-HS dirty page can generate even more HS content. As we can not evict HS pages
+ * Evicting an non-HS dirty page can generate even more HS content. As we cannot evict HS pages
* while checkpoint is operating on the HS file, we can end up in a situation where we exceed
- * the cache size limits.
+ * the cache size limit.
*/
if (conn->txn_global.checkpoint_running_hs && !WT_IS_HS(btree->dhandle) &&
__wt_cache_hs_dirty(session) && __wt_cache_full(session)) {
@@ -587,33 +602,29 @@ __evict_review(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_flags, bool
if (F_ISSET(session, WT_SESSION_NO_RECONCILE))
return (__wt_set_return(session, EBUSY));
- /*
- * If the page is dirty, reconcile it to decide if we can evict it.
- *
- * If we have an exclusive lock (we're discarding the tree), assert there are no updates we
- * cannot read.
- *
- * Don't set any other flags for internal pages: there are no update lists to be saved and
- * restored, changes can't be written into the history store table, nor can we re-create
- * internal pages in memory.
- *
- * For leaf pages:
- *
- * In-memory pages are a known configuration.
- *
- * Set the update/restore flag, so reconciliation will write blocks it can write and create a
- * list of skipped updates for blocks it cannot write, along with disk images. This is how
- * eviction of active, huge pages works: we take a big page and reconcile it into blocks, some
- * of which we write and discard, the rest of which we re-create as smaller in-memory pages,
- * (restoring the updates that stopped us from writing the block), and inserting the whole mess
- * into the page's parent. Set the flag in all cases because the incremental cost of
- * update/restore in reconciliation is minimal, eviction shouldn't have picked a page where
- * update/restore is necessary, absent some cache pressure. It's possible updates occurred after
- * we selected this page for eviction, but it's unlikely and we don't try and manage that risk.
- *
- * Additionally, if we aren't trying to free space in the cache, scrub the page and keep it in
- * memory.
- */
+ return (0);
+}
+
+/*
+ * __evict_reconcile --
+ * Reconcile the page for eviction.
+ */
+static int
+__evict_reconcile(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_flags)
+{
+ WT_BTREE *btree;
+ WT_CACHE *cache;
+ WT_CONNECTION_IMPL *conn;
+ WT_DECL_RET;
+ uint32_t flags;
+ bool closing, is_eviction_thread, snapshot_acquired, use_snapshot_for_app_thread;
+
+ btree = S2BT(session);
+ conn = S2C(session);
+ flags = WT_REC_EVICT;
+ closing = FLD_ISSET(evict_flags, WT_EVICT_CALL_CLOSING);
+ snapshot_acquired = false;
+
cache = conn->cache;
/*
@@ -623,22 +634,36 @@ __evict_review(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_flags, bool
if (FLD_ISSET(evict_flags, WT_EVICT_CALL_URGENT))
LF_SET(WT_REC_CALL_URGENT);
+ /*
+ * If we have an exclusive lock (we're discarding the tree), assert there are no updates we
+ * cannot read.
+ */
if (closing)
LF_SET(WT_REC_VISIBILITY_ERR);
+ /*
+ * Don't set any other flags for internal pages: there are no update lists to be saved and
+ * restored, changes can't be written into the history store table, nor can we re-create
+ * internal pages in memory.
+ *
+ * Don't set any other flags for history store table as all the content is evictable.
+ */
else if (F_ISSET(ref, WT_REF_FLAG_INTERNAL) || WT_IS_HS(btree->dhandle))
;
- else if (WT_SESSION_BTREE_SYNC(session) && !WT_IS_METADATA(btree->dhandle))
- LF_SET(WT_REC_HS);
+ /* Always do update restore for in-memory database. */
else if (F_ISSET(conn, WT_CONN_IN_MEMORY))
LF_SET(WT_REC_IN_MEMORY | WT_REC_SCRUB);
+ /* For data store leaf pages, write the history to history store except for metadata. */
else if (!WT_IS_METADATA(btree->dhandle)) {
LF_SET(WT_REC_HS);
/*
- * Scrub if we're supposed to or toss it in sometimes if we are in debugging mode.
+ * Scrub and we're supposed to or toss it in sometimes if we are in debugging mode.
+ *
+ * Note that don't scrub if checkpoint is running on the tree.
*/
- if (F_ISSET(cache, WT_CACHE_EVICT_SCRUB) ||
- (F_ISSET(cache, WT_CACHE_EVICT_DEBUG_MODE) && __wt_random(&session->rnd) % 3 == 0))
+ if (!WT_SESSION_BTREE_SYNC(session) &&
+ (F_ISSET(cache, WT_CACHE_EVICT_SCRUB) ||
+ (F_ISSET(cache, WT_CACHE_EVICT_DEBUG_MODE) && __wt_random(&session->rnd) % 3 == 0)))
LF_SET(WT_REC_SCRUB);
}
@@ -758,12 +783,8 @@ __evict_review(WT_SESSION_IMPL *session, WT_REF *ref, uint32_t evict_flags, bool
* Success: assert that the page is clean or reconciliation was configured to save updates.
*/
WT_ASSERT(session,
- !__wt_page_is_modified(page) || LF_ISSET(WT_REC_HS | WT_REC_IN_MEMORY) ||
+ !__wt_page_is_modified(ref->page) || LF_ISSET(WT_REC_HS | WT_REC_IN_MEMORY) ||
WT_IS_METADATA(btree->dhandle));
- /* Fail 0.1% of the time. */
- if (__wt_failpoint(session, WT_TIMING_STRESS_FAILPOINT_EVICTION_FAIL_AFTER_RECONCILIATION, 10))
- return (EBUSY);
-
return (0);
}
diff --git a/src/third_party/wiredtiger/test/format/failure_configs/CONFIG.WT-9751 b/src/third_party/wiredtiger/test/format/failure_configs/CONFIG.WT-9751
new file mode 100644
index 00000000000..8ff2a91c49d
--- /dev/null
+++ b/src/third_party/wiredtiger/test/format/failure_configs/CONFIG.WT-9751
@@ -0,0 +1,138 @@
+############################################
+# RUN PARAMETERS: V3
+############################################
+assert.read_timestamp=0
+assert.write_timestamp=0
+backup=0
+backup.incremental=off
+backup.incr_granularity=8115
+block_cache=0
+block_cache.cache_on_checkpoint=0
+block_cache.cache_on_writes=0
+block_cache.size=53
+btree.compression=none
+cache=336
+cache.evict_max=2
+cache.minimum=40
+checkpoint=on
+checkpoint.log_size=79
+checkpoint.wait=40
+disk.data_extend=0
+disk.direct_io=0
+disk.encryption=none
+disk.mmap=1
+disk.mmap_all=0
+format.abort=0
+format.independent_thread_rng=0
+format.major_timeout=0
+import=0
+logging=0
+logging.compression=none
+logging.file_max=481991
+logging.prealloc=0
+logging.remove=1
+ops.alter=0
+ops.compaction=0
+ops.hs_cursor=0
+ops.prepare=0
+ops.random_cursor=1
+ops.salvage=0
+ops.verify=1
+quiet=1
+runs.in_memory=0
+runs.ops=0
+runs.rows=100000
+runs.source=table
+runs.tables=3
+runs.threads=6
+runs.timer=1
+runs.type=variable-length column-store
+runs.verify_failure_dump=0
+statistics=0
+statistics.server=1
+stress.aggressive_sweep=0
+stress.checkpoint=0
+stress.checkpoint_evict_page=0
+stress.checkpoint_reserved_txnid_delay=0
+stress.checkpoint_prepare=0
+stress.evict_reposition=0
+stress.failpoint_eviction_fail_after_reconciliation=1
+stress.failpoint_hs_delete_key_from_ts=0
+stress.hs_checkpoint_delay=0
+stress.hs_search=0
+stress.hs_sweep=0
+stress.split_1=0
+stress.split_2=0
+stress.split_3=0
+stress.split_4=0
+stress.split_5=0
+stress.split_6=0
+stress.split_7=0
+transaction.implicit=0
+transaction.timestamps=1
+wiredtiger.config=off
+wiredtiger.rwlock=0
+wiredtiger.leak_memory=0
+############################################
+# TABLE PARAMETERS: table 1
+############################################
+table1.btree.dictionary=1
+table1.btree.huffman_value=0
+table1.btree.internal_key_truncation=1
+table1.btree.internal_page_max=17
+table1.btree.leaf_page_max=9
+table1.btree.memory_page_max=8
+table1.btree.repeat_data_pct=87
+table1.btree.split_pct=60
+table1.btree.value_max=1299
+table1.btree.value_min=12
+table1.disk.checksum=on
+table1.disk.firstfit=0
+table1.ops.pct.delete=77
+table1.ops.pct.insert=4
+table1.ops.pct.modify=12
+table1.ops.pct.read=1
+table1.ops.pct.write=6
+table1.ops.truncate=1
+############################################
+# TABLE PARAMETERS: table 2
+############################################
+table2.btree.dictionary=0
+table2.btree.huffman_value=0
+table2.btree.internal_key_truncation=1
+table2.btree.internal_page_max=14
+table2.btree.leaf_page_max=11
+table2.btree.memory_page_max=7
+table2.btree.repeat_data_pct=89
+table2.btree.split_pct=67
+table2.btree.value_max=3716
+table2.btree.value_min=11
+table2.disk.checksum=unencrypted
+table2.disk.firstfit=0
+table2.ops.pct.delete=20
+table2.ops.pct.insert=1
+table2.ops.pct.modify=7
+table2.ops.pct.read=72
+table2.ops.pct.write=0
+table2.ops.truncate=1
+############################################
+# TABLE PARAMETERS: table 3
+############################################
+table3.btree.dictionary=0
+table3.btree.huffman_value=0
+table3.btree.internal_key_truncation=1
+table3.btree.internal_page_max=17
+table3.btree.leaf_page_max=12
+table3.btree.memory_page_max=9
+table3.btree.repeat_data_pct=12
+table3.btree.split_pct=54
+table3.btree.value_max=2929
+table3.btree.value_min=8
+table3.disk.checksum=on
+table3.disk.firstfit=0
+table3.ops.pct.delete=34
+table3.ops.pct.insert=45
+table3.ops.pct.modify=1
+table3.ops.pct.read=0
+table3.ops.pct.write=20
+table3.ops.truncate=1