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

Break up __rec_append_orig_value into two functions to simplify code

    • Type: Icon: Improvement Improvement
    • Resolution: Unresolved
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None

      This part of the code has become very complicated in __rec_upd_select because we want to reuse __rec_append_orig_value instead of directly appending the onpage value here:

      else if (select_tw->stop_ts != WT_TS_NONE || select_tw->stop_txn != WT_TXN_NONE) {
                  /* If we only have a tombstone in the update list, we must have an ondisk value. */
                  WT_ASSERT(session,
                    vpack != NULL && tombstone != NULL && !vpack->tw.prepare && last_upd->next == NULL);
                  /*
                   * It's possible to have a tombstone as the only update in the update list. If we
                   * reconciled before with only a single update and then read the page back into cache,
                   * we'll have an empty update list. And applying a delete on top of that will result in
                   * ONLY a tombstone in the update list.
                   *
                   * In this case, we should leave the selected update unset to indicate that we want to
                   * keep the same on-disk value but set the stop time point to indicate that the validity
                   * window ends when this tombstone started.
                   *
                   * FIXME-WT-6557: no need to check this after WT-6557 is done as the tombstone will be
                   * freed when it is written to the disk image in the previous eviction.
                   */
                  if (!F_ISSET(tombstone, WT_UPDATE_RESTORED_FROM_DS | WT_UPDATE_RESTORED_FROM_HS)) {
                      WT_ERR(__rec_append_orig_value(session, page, tombstone, vpack));
      
                      /*
                       * We may have updated the global transaction concurrently and the tombstone is now
                       * globally visible. In this case, the on page value is not appended. Verify that.
                       */
                      if (last_upd->next != NULL) {
                          WT_ASSERT(session,
                            last_upd->next->txnid == vpack->tw.start_txn &&
                              last_upd->next->start_ts == vpack->tw.start_ts &&
                              last_upd->next->type == WT_UPDATE_STANDARD && last_upd->next->next == NULL);
                          upd_select->upd = last_upd->next;
                          WT_TIME_WINDOW_SET_START(select_tw, last_upd->next);
                      } else {
                          /*
                           * If the tombstone is aborted concurrently, we should still have appended the
                           * onpage value.
                           */
                          WT_ASSERT(session,
                            tombstone->txnid != WT_TXN_ABORTED &&
                              __wt_txn_upd_visible_all(session, tombstone) && upd_select->upd == NULL);
                          upd_select->upd = tombstone;
                      }
                  }
      

      We can simply this code by always append the onpage value in this section. To be able to also reuse some code. We can break up __rec_append_orig_value into two separate function. The first function decides whether we should append the onpage value. The second function just append the onpage value to the end of the update chain. We can reuse the second function here in this section of code and avoiding all this complex logic.

            Assignee:
            backlog-server-storage-engines [DO NOT USE] Backlog - Storage Engines Team
            Reporter:
            chenhao.qu@mongodb.com Chenhao Qu
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: