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

Redundant page lookups hurt performance

Details

    Description

      The function btr_cur_t::search_leaf() that was refactored in MDEV-30400 is executing redundant page lookups when it would suffice to upgrade an already held index root/leaf page latch to exclusive.

      monty noticed this while working on MDEV-34571.

      I can reproduce the anomaly with the following test case:

      --source include/have_innodb.inc
      		 
       
      		 
      SET GLOBAL innodb_max_purge_lag_wait=0;
      		 
      CREATE TEMPORARY TABLE t1(a SMALLINT PRIMARY KEY) ENGINE=InnoDB;
      		 
      BEGIN;
      		 
      SET @read0=
      		 
      (SELECT CAST(variable_value AS INTEGER) FROM information_schema.global_status
      		 
      WHERE variable_name = 'innodb_buffer_pool_read_requests');
      		 
      INSERT INTO t1 VALUES (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
      		 
      SET @read1=
      		 
      (SELECT CAST(variable_value AS INTEGER) FROM information_schema.global_status
      		 
      WHERE variable_name = 'innodb_buffer_pool_read_requests');
      		 
      INSERT INTO t1 VALUES (10),(11),(12),(13),(14),(15),(16),(17),(18),(19);
      		 
      SET @read2=
      		 
      (SELECT CAST(variable_value AS INTEGER) FROM information_schema.global_status
      		 
      WHERE variable_name = 'innodb_buffer_pool_read_requests');
      		 
      COMMIT;
      		 
       
      		 
      DROP TABLE t1;
      		 
      SELECT @read1-@read0, @read2-@read1;

      Without my fix, the last SELECT would report 33 and 34. With my fix, the numbers would reduce by 10, corresponding to the number of inserted rows. The remaining numbers (23 and 24) roughly correspond to writing one undo log record and one index record for each row.

      We could do significantly better if we retained the mini-transaction open between individual row operations (MDEV-16232).

      Attachments

        Issue Links

          blocks

          Bug - A problem which impairs or prevents the functions of the product. MDEV-34431 More fine grained control of spin loops could be useful

          • Critical - Crashes, loss of data, severe memory leak.
          • Stalled
          is blocked by

          Bug - A problem which impairs or prevents the functions of the product. MDEV-34759 buf_page_get_low() is unnecessarily acquiring exclusive latch on secondary index pages

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed
          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-35125 Unnecessary buf_pool.page_hash lookups

          • Major - Major loss of function.
          • Closed

          Task - A task that needs to be done. MDEV-16232 Use fewer mini-transactions

          • Major - Major loss of function.
          • Stalled

          Bug - A problem which impairs or prevents the functions of the product. MDEV-30400 Assertion `height == btr_page_get_level(page_cur_get_page(page_cursor))' failed in btr_cur_search_to_nth_level on INSERT

          • Blocker - Blocks development and/or testing work, production could not run.
          • Closed

          Task - A task that needs to be done. MDEV-34571 Add page accessed and pages read from disk to table_stats

          • Major - Major loss of function.
          • Closed

          Activity

            Ascending order - Click to sort in descending order
            marko Marko Mäkelä added a comment -

            My fix will extend the use of the page latch upgrade logic that I implemented in MDEV-34759.

            marko Marko Mäkelä added a comment - My fix will extend the use of the page latch upgrade logic that I implemented in MDEV-34759 .
            marko Marko Mäkelä added a comment -

            I searched the InnoDB code base for goto retry as well as for calls to mtr_t::rollback_to_savepoint() and mtr_t::release_last_page(). I found some more opportunities for optimization.

            • btr_latch_prev() had better retain a buffer-fix on both blocks while waiting for prev->page.lock.
            • row_merge_read_clustered_index() implements some logic that tries to yield for the purge of history. The usefulness of this would be good to recheck. Bottlenecks could have shifted due to MDEV-32050 and MDEV-34515.
            • Some SPATIAL INDEX operations could be improved as well, but I am reluctant to do it, because we already have rather fundamental correctness issues, such as MDEV-15284.
            marko Marko Mäkelä added a comment - I searched the InnoDB code base for goto retry as well as for calls to mtr_t::rollback_to_savepoint() and mtr_t::release_last_page() . I found some more opportunities for optimization. btr_latch_prev() had better retain a buffer-fix on both blocks while waiting for prev->page.lock . row_merge_read_clustered_index() implements some logic that tries to yield for the purge of history. The usefulness of this would be good to recheck. Bottlenecks could have shifted due to MDEV-32050 and MDEV-34515 . Some SPATIAL INDEX operations could be improved as well, but I am reluctant to do it, because we already have rather fundamental correctness issues, such as MDEV-15284 .
            debarun Debarun Banerjee added a comment -

            Nice work. Left some comments in [pull-3479|https://github.com/MariaDB/server/pull/3479|

            debarun Debarun Banerjee added a comment - Nice work. Left some comments in [pull-3479|https://github.com/MariaDB/server/pull/3479|

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              3 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.