Uploaded image for project: 'WiredTiger'
  1. WiredTiger
  2. WT-7521

Code cleanup: Evaluate adding a bool to mark saved checkpoint list invalid

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: In Progress
    • Priority: Major - P3
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 5.1 Required
    • Component/s: None
    • Labels:
      None

      Description

      This ticket tracks some of the work required after merging WT-7381 to refactor and clean up the code:

      Evaluate if we need to free the checkpoint list multiple times in __wt_checkpoint_close(). This came out of code review for WT-7381, following the comment. Initially there were a couple more places the initial change followed the pattern:

      __wt_meta_saved_ckptlist_free(session);
      // Take a checkpoint for an individual file
      __wt_meta_saved_ckptlist_free(session);
      

      The objective here is to clean the saved checkpoint list before checkpointing an individual file, and then clean after. This would make sure that as part of the system-wide checkpoint, a file slow paths by reading checkpoints from metadata, if that file was checkpointed outside the periodic system checkpointing.

      Over the course of refining the change, __wt_checkpoint_close is the only place left with this pattern, and might not actually need clearing the saved list after the checkpoint, because it will likely be destroyed by a btree close. So we can likely get rid of that extra call to __wt_checkpoint_close().

      On the other hand, if there is a need for such a pattern of freeing the saved checkpoint list before and after an operation, it would be easier to introduce a boolean, that can be set to true, as means to invalidate any existing list. Then when there is time to decide between going the slow path (reading metadata for the checkpoint list) vs the fast path (use the saved checkpoint list), we could destroy the saved list and go slow path.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              sulabh.mahajan Sulabh Mahajan
              Reporter:
              sulabh.mahajan Sulabh Mahajan
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated: