On Thu, Jan 3, 2013 at 1:03 AM, Michael Cahill <notifications@github.com>wrote:

> In src/btree/bt_evict.c:
>
> >  	 * Publish: there must be a barrier to ensure the return value is set
> >  	 * before the requesting thread wakes.
> >  	 */
> > -	WT_PUBLISH(request_session->syncop_ret,
> > -	    __evict_file(request_session, syncop));
> > +	if (syncop == WT_SYNC_DIRTY_DISABLE)
> > +		WT_PUBLISH(request_session->syncop_ret, 0);
> > +	else
> > +		WT_PUBLISH(request_session->syncop_ret,
> > +		    __evict_file(request_session, syncop));
>
> I don't understand what this achieves.
>
> When an application wants to start the internal phase of a checkpoint,
> there are some pages queued for LRU eviction. Some of them may be dirty,
> some not. Clearing the queue doesn't prevent new pages from being scheduled
> for LRU eviction and then becoming dirty. At that point, we need some other
> mechanism to be sure no thread will evict a dirty page.
>
> The basis of synchronization between threads is atomic updates to page
> states. If a thread locks a page, then checks that dirty eviction is
> permitted (the logic you've moved to __rec_review), I don't see how
> emptying the queue here makes things any safer.
>
> Maybe I'm being misled by the comment "drain the queue of eviction pages",
> and the intent is actually to wait for lru_count to go to zero -- in
> other words, for any reconciliation that already in progress to complete?
>
Yes, exactly, the point is that lru_count has to go to zero, and any
already running reconciliations/evictions complete before checkpoint
reconciles internal pages.

--keith

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Keith Bostic
keith.bostic@wiredtiger.com