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

Redundant check on prebuilt::n_rows_fetched overflow

Details

    Description

      In an InnoDB record cursor, there is a counter row_prebuilt_t::n_rows_fetched that counts records that have been fetched in a key range scan. The main purpose of this counter is to control whether to trigger a prefetch of a cache that is slated for removal in MDEV-16232.

      On 64-bit systems, this counter cannot overflow. If we overestimate that InnoDB could read 10¹² or 2⁴⁰ records per second in a range scan, it would take more than half a year of such a range scan before the counter would wrap around. An even more rigid limitation would be that InnoDB only supports 2³² pages per tablespace, and you could fit at most a few thousand or 2¹² records per page, that is, there can't be more than some 2⁴⁴ records per index.

      I would propose to revise an overflow check as follows, disabling it on 64-bit systems:

      diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc
      index 4504fabd4e0..b217f6d20fe 100644
      --- a/storage/innobase/row/row0sel.cc
      +++ b/storage/innobase/row/row0sel.cc
      @@ -4406,13 +4406,11 @@ row_search_mvcc(
       			goto func_exit;
       		}
       
      +#if SIZEOF_SIZE_T < 8
      +		if (UNIV_LIKELY(~prebuilt->n_rows_fetched))
      +#endif
       		prebuilt->n_rows_fetched++;
       
      -		if (prebuilt->n_rows_fetched > 1000000000) {
      -			/* Prevent wrap-over */
      -			prebuilt->n_rows_fetched = 500000000;
      -		}
      -
       		mode = pcur->search_mode;
       	}
       
      

      Attachments

        Issue Links

          Activity

            marko Marko Mäkelä created issue -
            marko Marko Mäkelä made changes -
            Field Original Value New Value
            marko Marko Mäkelä made changes -
            Description In an InnoDB record cursor, there is a counter {{row_prebuilt_t::n_rows_fetched}} that counts records that have been fetched in a key range scan. The main purpose of this counter is to control whether to trigger a prefetch of a cache that is slated for removal in MDEV-16232.

            On 64-bit systems, this counter cannot overflow. If we overestimate that InnoDB could read 10¹² or 2⁴⁰ records per second in a range scan, it would take more than half a year of such a range scan before the counter would wrap around. An even more rigid limitation would be that InnoDB only supports 2³² pages per tablespace, and you could fit at most a few thousand records per page, that is, there can't be more than some 2³⁶ records per index.

            I would propose to revise an overflow check as follows, disabling it on 64-bit systems:
            {code:c++}
            diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc
            index 4504fabd4e0..b217f6d20fe 100644
            --- a/storage/innobase/row/row0sel.cc
            +++ b/storage/innobase/row/row0sel.cc
            @@ -4406,13 +4406,11 @@ row_search_mvcc(
              goto func_exit;
              }
             
            +#if SIZEOF_SIZE_T < 8
            + if (UNIV_LIKELY(~prebuilt->n_rows_fetched))
            +#endif
              prebuilt->n_rows_fetched++;
             
            - if (prebuilt->n_rows_fetched > 1000000000) {
            - /* Prevent wrap-over */
            - prebuilt->n_rows_fetched = 500000000;
            - }
            -
              mode = pcur->search_mode;
              }
             
            {code}
            In an InnoDB record cursor, there is a counter {{row_prebuilt_t::n_rows_fetched}} that counts records that have been fetched in a key range scan. The main purpose of this counter is to control whether to trigger a prefetch of a cache that is slated for removal in MDEV-16232.

            On 64-bit systems, this counter cannot overflow. If we overestimate that InnoDB could read 10¹² or 2⁴⁰ records per second in a range scan, it would take more than half a year of such a range scan before the counter would wrap around. An even more rigid limitation would be that InnoDB only supports 2³² pages per tablespace, and you could fit at most a few thousand or 2¹² records per page, that is, there can't be more than some 2⁴⁴ records per index.

            I would propose to revise an overflow check as follows, disabling it on 64-bit systems:
            {code:c++}
            diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc
            index 4504fabd4e0..b217f6d20fe 100644
            --- a/storage/innobase/row/row0sel.cc
            +++ b/storage/innobase/row/row0sel.cc
            @@ -4406,13 +4406,11 @@ row_search_mvcc(
              goto func_exit;
              }
             
            +#if SIZEOF_SIZE_T < 8
            + if (UNIV_LIKELY(~prebuilt->n_rows_fetched))
            +#endif
              prebuilt->n_rows_fetched++;
             
            - if (prebuilt->n_rows_fetched > 1000000000) {
            - /* Prevent wrap-over */
            - prebuilt->n_rows_fetched = 500000000;
            - }
            -
              mode = pcur->search_mode;
              }
             
            {code}
            marko Marko Mäkelä made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            marko Marko Mäkelä made changes -
            Assignee Marko Mäkelä [ marko ] Vladislav Lesin [ vlad.lesin ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            vlad.lesin Vladislav Lesin made changes -
            Assignee Vladislav Lesin [ vlad.lesin ] Marko Mäkelä [ marko ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            marko Marko Mäkelä made changes -
            issue.field.resolutiondate 2024-01-30 12:30:25.0 2024-01-30 12:30:24.768
            marko Marko Mäkelä made changes -
            Fix Version/s 10.4.34 [ 29625 ]
            Fix Version/s 10.5.25 [ 29626 ]
            Fix Version/s 10.6.18 [ 29627 ]
            Fix Version/s 10.11.8 [ 29630 ]
            Fix Version/s 11.0.6 [ 29628 ]
            Fix Version/s 11.1.5 [ 29629 ]
            Fix Version/s 11.2.4 [ 29631 ]
            Fix Version/s 11.3.3 [ 29632 ]
            Fix Version/s 11.4.2 [ 29633 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            Fix Version/s 10.6 [ 24028 ]
            Fix Version/s 10.11 [ 27614 ]
            Fix Version/s 11.0 [ 28320 ]
            Fix Version/s 11.1 [ 28549 ]
            Fix Version/s 11.3 [ 28565 ]
            Fix Version/s 11.2 [ 28603 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 11.3.3 [ 29632 ]

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              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.