diff options
author | Alex Gorrod <alexander.gorrod@mongodb.com> | 2016-07-01 15:11:34 +1000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-07-01 15:11:34 +1000 |
commit | c74568b31a5fa834349a91ec4295d9cc1e49c9b7 (patch) | |
tree | 3697189071b8ce895b7d030a8c5dd72bc990f9cc | |
parent | 552a33b5bdb4f4d6e561c603a33ccb58a7ee9eca (diff) | |
download | mongo-c74568b31a5fa834349a91ec4295d9cc1e49c9b7.tar.gz |
WT-2733 Backport fixes for races between eviction and dead handle cleanup (#2841)
* WT-2313 Wrap clearing of WT_DHANDLE_OPEN with eviction exclusive calls.
(cherry picked from commit f4cc13a6dffda784be1fa33f8b46fefc5afab13e)
* WT-2434 Fix a race between forced drops and sweep.
(cherry picked from commit f6d0fa3645eb3f2b9932ffac9c287bba52052e04)
* Remove a stray line that was added during backport.
-rw-r--r-- | src/conn/conn_dhandle.c | 57 | ||||
-rw-r--r-- | src/conn/conn_sweep.c | 16 | ||||
-rw-r--r-- | src/evict/evict_lru.c | 43 |
3 files changed, 60 insertions, 56 deletions
diff --git a/src/conn/conn_dhandle.c b/src/conn/conn_dhandle.c index fed0012b211..1bb8e37249f 100644 --- a/src/conn/conn_dhandle.c +++ b/src/conn/conn_dhandle.c @@ -247,30 +247,6 @@ err: WT_TRET(__wt_rwlock_destroy(session, &dhandle->rwlock)); } /* - * __conn_dhandle_mark_dead -- - * Mark a data handle dead. - */ -static int -__conn_dhandle_mark_dead(WT_SESSION_IMPL *session) -{ - bool evict_reset; - - WT_ASSERT(session, F_ISSET(session, WT_SESSION_LOCKED_HANDLE_LIST)); - - /* - * Handle forced discard (e.g., when dropping a file). - * - * We need exclusive access to the file -- disable ordinary - * eviction and drain any blocks already queued. - */ - WT_RET(__wt_evict_file_exclusive_on(session, &evict_reset)); - F_SET(session->dhandle, WT_DHANDLE_DEAD); - if (evict_reset) - __wt_evict_file_exclusive_off(session); - return (0); -} - -/* * __wt_conn_btree_sync_and_close -- * Sync and close the underlying btree handle. */ @@ -280,10 +256,11 @@ __wt_conn_btree_sync_and_close(WT_SESSION_IMPL *session, bool final, bool force) WT_BTREE *btree; WT_DATA_HANDLE *dhandle; WT_DECL_RET; - bool no_schema_lock; + bool evict_reset, marked_dead, no_schema_lock; dhandle = session->dhandle; btree = S2BT(session); + evict_reset = marked_dead = false; if (!F_ISSET(dhandle, WT_DHANDLE_OPEN)) return (0); @@ -309,6 +286,13 @@ __wt_conn_btree_sync_and_close(WT_SESSION_IMPL *session, bool final, bool force) __wt_spin_lock(session, &dhandle->close_lock); /* + * Ensure we aren't racing with the eviction server; inside the close + * lock so threads won't race setting/clearing the tree's "no eviction" + * flag. + */ + WT_ERR(__wt_evict_file_exclusive_on(session, &evict_reset)); + + /* * The close can fail if an update cannot be written, return the EBUSY * error to our caller for eventual retry. * @@ -319,20 +303,31 @@ __wt_conn_btree_sync_and_close(WT_SESSION_IMPL *session, bool final, bool force) * invalid if the mapping is closed. */ if (!F_ISSET(btree, - WT_BTREE_SALVAGE | WT_BTREE_UPGRADE | WT_BTREE_VERIFY)) - WT_ERR(force && (btree->bm == NULL || btree->bm->map == NULL) ? - __conn_dhandle_mark_dead(session) : - __wt_checkpoint_close(session, final)); + WT_BTREE_SALVAGE | WT_BTREE_UPGRADE | WT_BTREE_VERIFY)) { + if (force && (btree->bm == NULL || btree->bm->map == NULL)) { + F_SET(session->dhandle, WT_DHANDLE_DEAD); + marked_dead = true; + } + if (!marked_dead || final) + WT_ERR(__wt_checkpoint_close(session, final)); + } WT_TRET(__wt_btree_close(session)); - if (!force || final) { + + /* + * If we marked a handle dead it will be closed by sweep, via + * another call to sync and close. + */ + if (!marked_dead) { F_CLR(dhandle, WT_DHANDLE_OPEN); if (dhandle->checkpoint == NULL) --S2C(session)->open_btree_count; } F_CLR(btree, WT_BTREE_SPECIAL_FLAGS); -err: __wt_spin_unlock(session, &dhandle->close_lock); +err: if (evict_reset) + __wt_evict_file_exclusive_off(session); + __wt_spin_unlock(session, &dhandle->close_lock); if (no_schema_lock) F_CLR(session, WT_SESSION_NO_SCHEMA_LOCK); diff --git a/src/conn/conn_sweep.c b/src/conn/conn_sweep.c index 8854f5e0592..a89b2f5ca6f 100644 --- a/src/conn/conn_sweep.c +++ b/src/conn/conn_sweep.c @@ -64,11 +64,9 @@ __sweep_expire_one(WT_SESSION_IMPL *session) WT_BTREE *btree; WT_DATA_HANDLE *dhandle; WT_DECL_RET; - bool evict_reset; btree = S2BT(session); dhandle = session->dhandle; - evict_reset = false; /* * Acquire an exclusive lock on the handle and mark it dead. @@ -92,19 +90,13 @@ __sweep_expire_one(WT_SESSION_IMPL *session) !__wt_txn_visible_all(session, btree->rec_max_txn)) goto err; - /* Ensure that we aren't racing with the eviction server */ - WT_ERR(__wt_evict_file_exclusive_on(session, &evict_reset)); - /* - * Mark the handle as dead and close the underlying file - * handle. Closing the handle decrements the open file count, - * meaning the close loop won't overrun the configured minimum. + * Mark the handle dead and close the underlying file handle. + * Closing the handle decrements the open file count, meaning the close + * loop won't overrun the configured minimum. */ ret = __wt_conn_btree_sync_and_close(session, false, true); - if (evict_reset) - __wt_evict_file_exclusive_off(session); - err: WT_TRET(__wt_writeunlock(session, dhandle->rwlock)); return (ret); @@ -171,7 +163,7 @@ __sweep_discard_trees(WT_SESSION_IMPL *session, u_int *dead_handlesp) !F_ISSET(dhandle, WT_DHANDLE_DEAD)) continue; - /* If the handle is marked "dead", flush it from cache. */ + /* If the handle is marked dead, flush it from cache. */ WT_WITH_DHANDLE(session, dhandle, ret = __wt_conn_btree_sync_and_close(session, false, false)); diff --git a/src/evict/evict_lru.c b/src/evict/evict_lru.c index 47917289503..dc64a55091d 100644 --- a/src/evict/evict_lru.c +++ b/src/evict/evict_lru.c @@ -729,31 +729,35 @@ __wt_evict_file_exclusive_on(WT_SESSION_IMPL *session, bool *evict_resetp) { WT_BTREE *btree; WT_CACHE *cache; + WT_DECL_RET; WT_EVICT_ENTRY *evict; u_int i, elem; btree = S2BT(session); cache = S2C(session)->cache; - /* - * If the file isn't evictable, there's no work to do. - */ - if (F_ISSET(btree, WT_BTREE_NO_EVICTION)) { - *evict_resetp = false; + *evict_resetp = false; + /* If the file was never evictable, there's no work to do. */ + if (F_ISSET(btree, WT_BTREE_NO_EVICTION)) return (0); - } - *evict_resetp = true; /* * Hold the walk lock to set the "no eviction" flag: no new pages from * the file will be queued for eviction after this point. */ __wt_spin_lock(session, &cache->evict_walk_lock); - F_SET(btree, WT_BTREE_NO_EVICTION); + if (!F_ISSET(btree, WT_BTREE_NO_EVICTION)) { + F_SET(btree, WT_BTREE_NO_EVICTION); + *evict_resetp = true; + } __wt_spin_unlock(session, &cache->evict_walk_lock); + /* If some other operation has disabled eviction, we're done. */ + if (!*evict_resetp) + return (0); + /* Clear any existing LRU eviction walk for the file. */ - WT_RET(__evict_request_walk_clear(session)); + WT_ERR(__evict_request_walk_clear(session)); /* Hold the evict lock to remove any queued pages from this file. */ __wt_spin_lock(session, &cache->evict_lock); @@ -776,6 +780,10 @@ __wt_evict_file_exclusive_on(WT_SESSION_IMPL *session, bool *evict_resetp) __wt_yield(); return (0); + +err: F_CLR(btree, WT_BTREE_NO_EVICTION); + *evict_resetp = false; + return (ret); } /* @@ -789,7 +797,14 @@ __wt_evict_file_exclusive_off(WT_SESSION_IMPL *session) btree = S2BT(session); - WT_ASSERT(session, btree->evict_ref == NULL); + /* + * We have seen subtle bugs with multiple threads racing to turn + * eviction on/off. Make races more likely in diagnostic builds. + */ + WT_DIAGNOSTIC_YIELD; + + WT_ASSERT(session, btree->evict_ref == NULL && + F_ISSET(btree, WT_BTREE_NO_EVICTION)); F_CLR(btree, WT_BTREE_NO_EVICTION); } @@ -952,6 +967,7 @@ __evict_walk(WT_SESSION_IMPL *session, uint32_t flags) conn = S2C(session); cache = S2C(session)->cache; + btree = NULL; dhandle = NULL; dhandle_locked = incr = false; retries = 0; @@ -1011,6 +1027,7 @@ retry: while (slot < max_entries && ret == 0) { (void)__wt_atomic_subi32( &dhandle->session_inuse, 1); incr = false; + cache->evict_file_next = NULL; } dhandle = TAILQ_NEXT(dhandle, q); } @@ -1065,6 +1082,9 @@ retry: while (slot < max_entries && ret == 0) { * exclusive access when a handle is being closed. */ if (!F_ISSET(btree, WT_BTREE_NO_EVICTION)) { + /* Remember the file to visit first, next loop. */ + cache->evict_file_next = dhandle; + WT_WITH_DHANDLE(session, dhandle, ret = __evict_walk_file(session, &slot, flags)); WT_ASSERT(session, session->split_gen == 0); @@ -1084,9 +1104,6 @@ retry: while (slot < max_entries && ret == 0) { } if (incr) { - /* Remember the file we should visit first, next loop. */ - cache->evict_file_next = dhandle; - WT_ASSERT(session, dhandle->session_inuse > 0); (void)__wt_atomic_subi32(&dhandle->session_inuse, 1); incr = false; |