Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor - P4
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: WT2.7.0
    • Labels:
      None
    • # Replies:
      4
    • Last comment by Customer:
      true

      Description

      Reference WT-1919.

      WT_PM_REC_REWRITE is a short-lived page state set in one specific path, when forced eviction of a leaf page fails because there's nothing much to write. The fact it must be a leaf page during eviction is because it is only set in a reconciliation call where WT_EVICT_UPDATE_RESTORE was set. It's short-lived because before we give up the exclusive lock on the page, we create a new page and swap it into place, and discard the page where WT_PM_REC_REWRITE was set.

      For these reasons...

      sync_file (the WT_SYNC_CHECKPOINT operation) should not be checking for WT_PM_REC_REWRITE, it should never see that flag as the checkpoint will be blocked from the leaf page until the eviction is complete.

      rec_root_write should not be checking for WT_PM_REC_REWRITE, it should never see that flag because it's writing internal pages, not leaf pages (and it's not evicting, anyway).

      rec_col_int should not be checking for WT_PM_REC_REWRITE, because any write of a page's column-store internal page parent will be blocked from evaluating the leaf child until the eviction is complete.

      rec_write_wrapup should not be checking for WT_PM_REC_REWRITE, because any previous reconciliation that set WT_PM_REC_REWRITE will have cleared that flag before this reconciliation could proceed.

      rec_write_wrapup_err should not be checking for WT_PM_REC_REWRITE because it's only set when a reconciliation is already considered successful.

      This pretty much removes the need for the WT_PM_REC_REWRITE state – the additional error handling makes me think we'd be better off getting rid of it and using WT_PM_REC_MULTIBLOCK with a single test, if we ended up with 1 block or several blocks.

        Issue Links

          Activity

          Hide
          keith.bostic Keith Bostic added a comment -

          Michael Cahill, I've pushed a set of changes for this (mostly reverting the changes for WT-1919, and with a different fix for the problem that WT-1919 was chasing).

          I still don't understand how it was possible for us to get into the state WT-1919 described: the "in-memory rewrite" page state should be replaced by a new WT_REF_MEM page where WT_PM_REC_MULTIBLOCK isn't set before the exclusive eviction lock is released, and with an impossibly low first_dirty_txn value to ensure the page is written, besides. All this can only happen if the page is dirty, of course, so the usual checkpoint/eviction dirty-page lockout is in force.

          Maybe the tree was different when WT-1919 went in, but I've done a spot check of the places I'd expect a problem, and it looks correct to me. Anyway, in summary, there's still some stuff here that needs an explanation, and I'm hoping you remember!

          Regardless, I think this change is a better fix for WT-1919, it removes a bunch of special-purpose code and ensures if this path is possible, the replacement blocks we create are consistent and correct for a merge into the parent page.

          Show
          keith.bostic Keith Bostic added a comment - Michael Cahill , I've pushed a set of changes for this (mostly reverting the changes for WT-1919 , and with a different fix for the problem that WT-1919 was chasing). I still don't understand how it was possible for us to get into the state WT-1919 described: the "in-memory rewrite" page state should be replaced by a new WT_REF_MEM page where WT_PM_REC_MULTIBLOCK isn't set before the exclusive eviction lock is released, and with an impossibly low first_dirty_txn value to ensure the page is written, besides. All this can only happen if the page is dirty, of course, so the usual checkpoint/eviction dirty-page lockout is in force. Maybe the tree was different when WT-1919 went in, but I've done a spot check of the places I'd expect a problem, and it looks correct to me. Anyway, in summary, there's still some stuff here that needs an explanation, and I'm hoping you remember! Regardless, I think this change is a better fix for WT-1919 , it removes a bunch of special-purpose code and ensures if this path is possible, the replacement blocks we create are consistent and correct for a merge into the parent page.
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'}

          Message: WT-2108: merge WT_PM_REC_REWRITE into WT_PM_REC_MULTIBLOCK.

          This change reverts most of e099f42 and 52b8734.
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/d7c033ad940f612d9f64545df3459ce0da8754d8

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'} Message: WT-2108 : merge WT_PM_REC_REWRITE into WT_PM_REC_MULTIBLOCK. This change reverts most of e099f42 and 52b8734. Branch: develop https://github.com/wiredtiger/wiredtiger/commit/d7c033ad940f612d9f64545df3459ce0da8754d8
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'}

          Message: WT-1919, WT-2108: The code to create the information for an rewrite of
          a single block in memory wasn't setting the multiblock key; should its
          parent page somehow be written before the leaf page is rewritten and
          attempt to merge the child page's information into the parent, we could
          drop core. I still don't see the path where that's possible in the
          current system, but it's the right thing to do regardless, the block
          should always be consistent.
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/cb8e834c550da6ca45b0551c8d212194ef95adee

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'} Message: WT-1919 , WT-2108 : The code to create the information for an rewrite of a single block in memory wasn't setting the multiblock key; should its parent page somehow be written before the leaf page is rewritten and attempt to merge the child page's information into the parent, we could drop core. I still don't see the path where that's possible in the current system, but it's the right thing to do regardless, the block should always be consistent. Branch: develop https://github.com/wiredtiger/wiredtiger/commit/cb8e834c550da6ca45b0551c8d212194ef95adee
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'}

          Message: Merge pull request #2192 from wiredtiger/wt-2108

          WT-2108 Rework in-memory page rewrite support (WT_PM_REC_REWRITE).
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/6a565bc6fd653db6704fd96a2975a1590178beba

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'} Message: Merge pull request #2192 from wiredtiger/wt-2108 WT-2108 Rework in-memory page rewrite support (WT_PM_REC_REWRITE). Branch: develop https://github.com/wiredtiger/wiredtiger/commit/6a565bc6fd653db6704fd96a2975a1590178beba

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since reply:
                1 year, 37 weeks, 2 days ago
                Date of 1st Reply: