WT-2774 minor cleanups/improvements (#2881)

* Enhance the information reported by __wt_cache_dump(), specifically, add more clean/dirty information. * Rework e41c3e5, do reconciliation cleanup when reconciliation finishes, that way we don't repeat work and/or do the work in multiple places.
wiredtiger · Jul 18, 2016 · 971c34a · 971c34a
1 parent 2718c0b
commit 971c34a
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 63 deletions.
diff --git a/src/evict/evict_lru.c b/src/evict/evict_lru.c
@@ -669,14 +669,13 @@ __evict_pass(WT_SESSION_IMPL *session)
 		 * sleep, it's not something we can fix.
 		 */
 		if (pages_evicted == cache->pages_evict) {
-			WT_STAT_FAST_CONN_INCR(session,
-			    cache_eviction_server_slept);
 			/*
-			 * Back off if we aren't making progress: walks hold
-			 * the handle list lock, which blocks other operations
-			 * that can free space in cache, such as LSM discarding
-			 * handles.
+			 * Back off if we aren't making progress: walks hold the
+			 * handle list lock, blocking other operations that can
+			 * free space in cache, such as LSM discarding handles.
 			 */
+			WT_STAT_FAST_CONN_INCR(session,
+			    cache_eviction_server_slept);
 			__wt_sleep(0, WT_THOUSAND * (uint64_t)loop);
 			if (loop == 100) {
 				/*
@@ -1887,13 +1886,15 @@ __wt_cache_dump(WT_SESSION_IMPL *session, const char *ofile)
 	WT_DATA_HANDLE *dhandle, *saved_dhandle;
 	WT_PAGE *page;
 	WT_REF *next_walk;
-	uint64_t dirty_bytes, dirty_pages, intl_bytes, intl_pages;
-	uint64_t leaf_bytes, leaf_pages;
-	uint64_t max_dirty_bytes, max_intl_bytes, max_leaf_bytes, total_bytes;
+	uint64_t intl_bytes, intl_bytes_max, intl_dirty_bytes;
+	uint64_t intl_dirty_bytes_max, intl_dirty_pages, intl_pages;
+	uint64_t leaf_bytes, leaf_bytes_max, leaf_dirty_bytes;
+	uint64_t leaf_dirty_bytes_max, leaf_dirty_pages, leaf_pages;
+	uint64_t total_bytes, total_dirty_bytes;
 	size_t size;
 
 	conn = S2C(session);
-	total_bytes = 0;
+	total_bytes = total_dirty_bytes = 0;
 
 	if (ofile == NULL)
 		fp = stderr;
@@ -1909,9 +1910,10 @@ __wt_cache_dump(WT_SESSION_IMPL *session, const char *ofile)
 		    !F_ISSET(dhandle, WT_DHANDLE_OPEN))
 			continue;
 
-		dirty_bytes = dirty_pages = intl_bytes = intl_pages = 0;
-		leaf_bytes = leaf_pages = 0;
-		max_dirty_bytes = max_intl_bytes = max_leaf_bytes = 0;
+		intl_bytes = intl_bytes_max = intl_dirty_bytes = 0;
+		intl_dirty_bytes_max = intl_dirty_pages = intl_pages = 0;
+		leaf_bytes = leaf_bytes_max = leaf_dirty_bytes = 0;
+		leaf_dirty_bytes_max = leaf_dirty_pages = leaf_pages = 0;
 
 		next_walk = NULL;
 		session->dhandle = dhandle;
@@ -1924,17 +1926,23 @@ __wt_cache_dump(WT_SESSION_IMPL *session, const char *ofile)
 			if (WT_PAGE_IS_INTERNAL(page)) {
 				++intl_pages;
 				intl_bytes += size;
-				max_intl_bytes = WT_MAX(max_intl_bytes, size);
+				intl_bytes_max = WT_MAX(intl_bytes_max, size);
+				if (__wt_page_is_modified(page)) {
+					++intl_dirty_pages;
+					intl_dirty_bytes += size;
+					intl_dirty_bytes_max =
+					    WT_MAX(intl_dirty_bytes_max, size);
+				}
 			} else {
 				++leaf_pages;
 				leaf_bytes += size;
-				max_leaf_bytes = WT_MAX(max_leaf_bytes, size);
-			}
-			if (__wt_page_is_modified(page)) {
-				++dirty_pages;
-				dirty_bytes += size;
-				max_dirty_bytes =
-				    WT_MAX(max_dirty_bytes, size);
+				leaf_bytes_max = WT_MAX(leaf_bytes_max, size);
+				if (__wt_page_is_modified(page)) {
+					++leaf_dirty_pages;
+					leaf_dirty_bytes += size;
+					leaf_dirty_bytes_max =
+					    WT_MAX(leaf_dirty_bytes_max, size);
+				}
 			}
 		}
 		session->dhandle = NULL;
@@ -1946,21 +1954,41 @@ __wt_cache_dump(WT_SESSION_IMPL *session, const char *ofile)
 			    dhandle->name, dhandle->checkpoint);
 		if (intl_pages != 0)
 			(void)fprintf(fp,
-			    "\t" "internal pages: %" PRIu64 " pages, %" PRIu64
-			    " max, %" PRIu64 "MB total\n",
-			    intl_pages, max_intl_bytes, intl_bytes >> 20);
+			    "\t" "internal: "
+			    "%" PRIu64 " pages, "
+			    "%" PRIu64 "MB, "
+			    "%" PRIu64 "/%" PRIu64 " clean/dirty pages, "
+			    "%" PRIu64 "/%" PRIu64 " clean/dirty MB, "
+			    "%" PRIu64 "MB max page, "
+			    "%" PRIu64 "MB max dirty page\n",
+			    intl_pages,
+			    intl_bytes >> 20,
+			    intl_pages - intl_dirty_pages,
+			    intl_dirty_pages,
+			    (intl_bytes - intl_dirty_bytes) >> 20,
+			    intl_dirty_bytes >> 20,
+			    intl_bytes_max >> 20,
+			    intl_dirty_bytes_max >> 20);
 		if (leaf_pages != 0)
 			(void)fprintf(fp,
-			    "\t" "leaf pages: %" PRIu64 " pages, %" PRIu64
-			    " max, %" PRIu64 "MB total\n",
-			    leaf_pages, max_leaf_bytes, leaf_bytes >> 20);
-		if (dirty_pages != 0)
-			(void)fprintf(fp,
-			    "\t" "dirty pages: %" PRIu64 " pages, %" PRIu64
-			    " max, %" PRIu64 "MB total\n",
-			    dirty_pages, max_dirty_bytes, dirty_bytes >> 20);
+			    "\t" "leaf: "
+			    "%" PRIu64 " pages, "
+			    "%" PRIu64 "MB, "
+			    "%" PRIu64 "/%" PRIu64 " clean/dirty pages, "
+			    "%" PRIu64 "/%" PRIu64 " clean/dirty MB, "
+			    "%" PRIu64 "MB max page, "
+			    "%" PRIu64 "MB max dirty page\n",
+			    leaf_pages,
+			    leaf_bytes >> 20,
+			    leaf_pages - leaf_dirty_pages,
+			    leaf_dirty_pages,
+			    (leaf_bytes - leaf_dirty_bytes) >> 20,
+			    leaf_dirty_bytes >> 20,
+			    leaf_bytes_max >> 20,
+			    leaf_dirty_bytes_max >> 20);
 
 		total_bytes += intl_bytes + leaf_bytes;
+		total_dirty_bytes += intl_dirty_bytes + leaf_dirty_bytes;
 	}
 	session->dhandle = saved_dhandle;
 
@@ -1972,10 +2000,13 @@ __wt_cache_dump(WT_SESSION_IMPL *session, const char *ofile)
 		total_bytes +=
 		    (total_bytes * (uint64_t)conn->cache->overhead_pct) / 100;
 	(void)fprintf(fp,
-	    "cache dump: total found = %" PRIu64
-	    "MB vs tracked inuse %" PRIu64 "MB\n",
-	    total_bytes >> 20, __wt_cache_bytes_inuse(conn->cache) >> 20);
+	    "cache dump: "
+	    "total found = %" PRIu64 "MB vs tracked inuse %" PRIu64 "MB\n"
+	    "total dirty bytes = %" PRIu64 "MB\n",
+	    total_bytes >> 20, __wt_cache_bytes_inuse(conn->cache) >> 20,
+	    total_dirty_bytes >> 20);
 	(void)fprintf(fp, "==========\n");
+
 	if (ofile != NULL && fclose(fp) != 0)
 		return (EIO);
 	return (0);

diff --git a/src/evict/evict_page.c b/src/evict/evict_page.c
@@ -187,18 +187,6 @@ err:		if (!closing)
 		WT_STAT_FAST_DATA_INCR(session, cache_eviction_fail);
 	}
 
-	/*
-	 * When application threads perform eviction, we don't want to cache
-	 * reconciliation structures.
-	 */
-	if (!F_ISSET(session, WT_SESSION_INTERNAL)) {
-		if (session->block_manager_cleanup != NULL)
-			WT_TRET(session->block_manager_cleanup(session));
-
-		if (session->reconcile_cleanup != NULL)
-			WT_TRET(session->reconcile_cleanup(session));
-	}
-
 	return (ret);
 }
 
@@ -397,8 +385,6 @@ __evict_review(
 	bool modified;
 
 	flags = WT_EVICTING;
-	if (closing)
-		LF_SET(WT_VISIBILITY_ERR);
 	*flagsp = flags;
 
 	/*
@@ -513,7 +499,9 @@ __evict_review(
 	 * pages, they don't have update lists that can be saved and restored.
 	 */
 	cache = S2C(session)->cache;
-	if (!closing && !WT_PAGE_IS_INTERNAL(page)) {
+	if (closing)
+		LF_SET(WT_VISIBILITY_ERR);
+	else if (!WT_PAGE_IS_INTERNAL(page)) {
 		if (F_ISSET(S2C(session), WT_CONN_IN_MEMORY))
 			LF_SET(WT_EVICT_IN_MEMORY | WT_EVICT_UPDATE_RESTORE);
 		else if (page->read_gen == WT_READGEN_OLDEST ||

diff --git a/src/reconcile/rec_write.c b/src/reconcile/rec_write.c
@@ -445,17 +445,32 @@ __wt_reconcile(WT_SESSION_IMPL *session,
 	}
 
 	/*
-	 * Clean up reconciliation resources: some workloads have millions of
-	 * boundary structures, and if associated with an application session
-	 * pulled into doing forced eviction, they won't be discarded for the
-	 * life of the session (or until session.reset is called). Discard all
-	 * of the reconciliation resources if an application thread, not doing
-	 * a checkpoint.
-	 */
-	__rec_bnd_cleanup(session, r,
-	    F_ISSET(session, WT_SESSION_INTERNAL) ||
-	    WT_SESSION_IS_CHECKPOINT(session) ? false : true);
+	 * When application threads perform eviction, don't cache block manager
+	 * or reconciliation structures (even across calls), we can have a
+	 * significant number of application threads doing eviction at the same
+	 * time with large items. We ignore checkpoints, once the checkpoint
+	 * completes, all unnecessary session resources will be discarded.
+	 *
+	 * Even in application threads doing checkpoints or in internal threads
+	 * doing any reconciliation, clean up reconciliation resources. Some
+	 * workloads have millions of boundary structures in a reconciliation
+	 * and we don't want to tie that memory down, even across calls.
+	 */
+	if (WT_SESSION_IS_CHECKPOINT(session) ||
+	    F_ISSET(session, WT_SESSION_INTERNAL))
+		__rec_bnd_cleanup(session, r, false);
+	else {
+		/*
+		 * Clean up the underlying block manager memory too: it's not
+		 * reconciliation, but threads discarding reconciliation
+		 * structures want to clean up the block manager's structures
+		 * as well, and there's no obvious place to do that.
+		 */
+		if (session->block_manager_cleanup != NULL)
+			WT_TRET(session->block_manager_cleanup(session));
 
+		WT_TRET(__rec_destroy_session(session));
+	}
 	WT_RET(ret);
 
 	/*

diff --git a/test/format/ops.c b/test/format/ops.c
@@ -342,15 +342,15 @@ snap_check(WT_CURSOR *cursor,
 		switch (g.type) {
 		case FIX:
 			testutil_die(ret,
-			    "snap_check: %" PRIu64 " search: "
+			    "snapshot-isolation: %" PRIu64 " search: "
 			    "expected {0x%02x}, found {0x%02x}",
 			    start->keyno,
 			    start->deleted ? 0 : *(uint8_t *)start->vdata,
 			    ret == WT_NOTFOUND ? 0 : *(uint8_t *)value->data);
 			/* NOTREACHED */
 		case ROW:
 			testutil_die(ret,
-			    "snap_check: %.*s search: "
+			    "snapshot-isolation: %.*s search: "
 			    "expected {%.*s}, found {%.*s}",
 			    (int)key->size, key->data,
 			    start->deleted ?
@@ -362,7 +362,7 @@ snap_check(WT_CURSOR *cursor,
 			/* NOTREACHED */
 		case VAR:
 			testutil_die(ret,
-			    "snap_check: %" PRIu64 " search: "
+			    "snapshot-isolation: %" PRIu64 " search: "
 			    "expected {%.*s}, found {%.*s}",
 			    start->keyno,
 			    start->deleted ?