Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-19051

Avoid unnecessary writing MLOG_INDEX_LOAD

Details

    Description

      MLOG_INDEX_LOAD redo log is written during online alter table. But MLOG_INDEX_LOAD
      is written for the root page even though the page is empty and for spatial index too. By fixing this, we can save some I/O for mariabackup
      and even during redo log recovery. (MDEV-12699). It also saves a few bytes in the redo log.

      Attachments

        Activity

          Ascending order - Click to sort in descending order
          thiru Thirunarayanan Balathandayuthapani added a comment -

          Below patch can fix the issue for un-necessary writing of MLOG_INDEX_LOAD redo log record.

          diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
          		 
          index 4f05899..3c5e715 100644
          		 
          --- a/storage/innobase/row/row0merge.cc
          		 
          +++ b/storage/innobase/row/row0merge.cc
          		 
          @@ -4559,6 +4559,7 @@ row_merge_build_indexes(
          		 
                  ulint total_index_blocks = 0;
          		 
                  double pct_cost=0;
          		 
                  double pct_progress=0;
          		 
          +       bool    is_empty_table = false;
          		 
           
          		 
                  DBUG_ENTER("row_merge_build_indexes");
          		 
           
          		 
          @@ -4637,6 +4638,7 @@ row_merge_build_indexes(
          		 
                  for (i = 0; i < n_indexes; i++) {
          		 
                          merge_files[i].fd = -1;
          		 
                          merge_files[i].offset = 0;
          		 
          +               merge_files[i].n_rec = 0;
          		 
                  }
          		 
           
          		 
                  total_static_cost = COST_BUILD_INDEX_STATIC * n_indexes + COST_READ_CLUSTERED_INDEX;
          		 
          @@ -4723,6 +4725,8 @@ row_merge_build_indexes(
          		 
                          total_index_blocks += merge_files[i].offset;
          		 
                  }
          		 
           
          		 
          +       is_empty_table = (merge_files[0].n_rec == 0);
          		 
          +
          		 
                  if (error != DB_SUCCESS) {
          		 
                          goto func_exit;
          		 
                  }
          		 
          @@ -4915,7 +4919,7 @@ row_merge_build_indexes(
          		 
                                  ut_ad(sort_idx->online_status
          		 
                                        == ONLINE_INDEX_COMPLETE);
          		 
                          } else {
          		 
          -                       if (flush_observer) {
          		 
          +                       if (flush_observer && !is_empty_table) {
          		 
                                          flush_observer->flush();
          		 
                                          row_merge_write_redo(indexes[i]);
          		 
                                  }
          		 
          @@ -5033,7 +5037,8 @@ row_merge_build_indexes(
          		 
                                  error = DB_INTERRUPTED;
          		 
                          }
          		 
          -               if (error == DB_SUCCESS && old_table != new_table) {
          		 
          +               if (error == DB_SUCCESS && old_table != new_table
          		 
          +                   && !is_empty_table) {
          		 
                                  for (const dict_index_t* index
          		 
                                               = dict_table_get_first_index(new_table);
          		 
                                       index != NULL;

          I tested the patch manually by adding ib_log as restart parameter in mtr test case.
          Tested for both PK(rebuild table) and index build.

          thiru Thirunarayanan Balathandayuthapani added a comment - Below patch can fix the issue for un-necessary writing of MLOG_INDEX_LOAD redo log record. diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc index 4f05899..3c5e715 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -4559,6 +4559,7 @@ row_merge_build_indexes( ulint total_index_blocks = 0; double pct_cost=0; double pct_progress=0; + bool is_empty_table = false;   DBUG_ENTER("row_merge_build_indexes");   @@ -4637,6 +4638,7 @@ row_merge_build_indexes( for (i = 0; i < n_indexes; i++) { merge_files[i].fd = -1; merge_files[i].offset = 0; + merge_files[i].n_rec = 0; }   total_static_cost = COST_BUILD_INDEX_STATIC * n_indexes + COST_READ_CLUSTERED_INDEX; @@ -4723,6 +4725,8 @@ row_merge_build_indexes( total_index_blocks += merge_files[i].offset; }   + is_empty_table = (merge_files[0].n_rec == 0); + if (error != DB_SUCCESS) { goto func_exit; } @@ -4915,7 +4919,7 @@ row_merge_build_indexes( ut_ad(sort_idx->online_status == ONLINE_INDEX_COMPLETE); } else { - if (flush_observer) { + if (flush_observer && !is_empty_table) { flush_observer->flush(); row_merge_write_redo(indexes[i]); } @@ -5033,7 +5037,8 @@ row_merge_build_indexes( error = DB_INTERRUPTED; } - if (error == DB_SUCCESS && old_table != new_table) { + if (error == DB_SUCCESS && old_table != new_table + && !is_empty_table) { for (const dict_index_t* index = dict_table_get_first_index(new_table); index != NULL; I tested the patch manually by adding ib_log as restart parameter in mtr test case. Tested for both PK(rebuild table) and index build.
          marko Marko Mäkelä added a comment -

          It looks like we never disable redo logging for creating spatial indexes, so we should not write MLOG_INDEX_LOAD records for them.

          marko Marko Mäkelä added a comment - It looks like we never disable redo logging for creating spatial indexes, so we should not write MLOG_INDEX_LOAD records for them.

          People

            thiru Thirunarayanan Balathandayuthapani
            thiru Thirunarayanan Balathandayuthapani
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Git Integration

                Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.