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

table_share used for reading statistical tables is not protected

Details

    Description

      While reading statistics for a table in function read_statistics_for_table_is_needed we don't protect the table_share fields and there is a chance that multiple threads would try to access it at the same time.

      To see this
      Lets add this patch to read_statistics_for_table

      +fprintf(stderr, "READ %p\n", current_thd); fflush(stderr);
      		 
      +sleep(10);
      		 
      +fprintf(stderr, "DONE %p\n", current_thd); fflush(stderr);
      		 
      +

      and the server output is

       
      		 
      READ 0x7fc870000b00 
      READ 0x7fc874000b00
      		 
      DONE 0x7fc870000b00
      		 
      DONE 0x7fc874000b00

      so different threads are entering the function at the same time.
      So i think we need an exclusive lock to protect the members of table_share

      Attachments

        Issue Links

          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-18899 Server crashes in Field::set_warning_truncated_wrong_value

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

          Bug - A problem which impairs or prevents the functions of the product. MDEV-19407 Assertion `field->table->stats_is_read' failed in is_eits_usable

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

          Bug - A problem which impairs or prevents the functions of the product. MDEV-29693 ANALYZE TABLE still flushes table definition cache when engine-independent statistics is used

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

          Activity

            Ascending order - Click to sort in descending order
            varun Varun Gupta (Inactive) added a comment -

            The test case

            set optimizer_use_condition_selectivity=4;
            		 
            set use_stat_tables=preferably;
            		 
            set histogram_size=255;
            		 
             
            		 
            create or replace table t1 (a int);
            		 
            insert into t1 values (1), (2);
            		 
             
            		 
            analyze table t1;
            		 
             
            		 
            FLUSH TABLES;
            		 
             
            		 
            connect (con1, localhost, root,,);
            		 
            connect (con2, localhost, root,,);
            		 
             
            		 
            connection con1;
            		 
            set use_stat_tables=preferably;
            		 
            --send select * from t1 where a  >= 1;
            		 
             
            		 
            connection con2;
            		 
            set use_stat_tables=preferably;
            		 
            select * from t1 where a  >= 1;
            		 
             
            		 
            connection con1;
            		 
            --reap
            		 
            connection con2;
            		 
             
            		 
            connection default;
            		 
            disconnect con1;
            		 
            disconnect con2;
            		 
             
            		 
            drop table t1;

            varun Varun Gupta (Inactive) added a comment - The test case set optimizer_use_condition_selectivity=4; set use_stat_tables=preferably; set histogram_size=255;   create or replace table t1 (a int ); insert into t1 values (1), (2);   analyze table t1;   FLUSH TABLES;   connect (con1, localhost, root,,); connect (con2, localhost, root,,);   connection con1; set use_stat_tables=preferably; --send select * from t1 where a >= 1;   connection con2; set use_stat_tables=preferably; select * from t1 where a >= 1;   connection con1; --reap connection con2;   connection default ; disconnect con1; disconnect con2;   drop table t1;
            svoj Sergey Vojtovich added a comment -

            Atomics based solution prototype:

            if (load(state, ACQUIRE) == STATS_ARE_READ)
            		 
              continue;
            		 
            if (!cas(state, STATS_NOT_READ, STATS_ARE_BEING_READ, RELAXED, RELAXED)
            		 
            {
            		 
              while (load(state, ACQUIRE) != STATS_ARE_READ) /* no-op */;
            		 
              continue;
            		 
            }
            		 
            ...read stats...
            		 
            store(state, STATS_ARE_READ, RELEASE);

            Also the following looks quite wrong:

                  if (thd->variables.optimizer_use_condition_selectivity > 3 &&
            		 
                      table_share && !table_share->stats_cb.histograms_are_read)
            		 
                  {
            		 
                    (void) read_histograms_for_table(thd, tl->table, stat_tables);
            		 
                    table_share->stats_cb.histograms_are_read= TRUE;
            		 
                  }
            		 
                  if (table_share->stats_cb.stats_is_read)
            		 
                    tl->table->histograms_are_read= TRUE;

            Shouldn't second if check histograms_are_read instead?

            svoj Sergey Vojtovich added a comment - Atomics based solution prototype: if (load(state, ACQUIRE) == STATS_ARE_READ) continue; if (!cas(state, STATS_NOT_READ, STATS_ARE_BEING_READ, RELAXED, RELAXED) { while (load(state, ACQUIRE) != STATS_ARE_READ) /* no-op */; continue; } ...read stats... store(state, STATS_ARE_READ, RELEASE); Also the following looks quite wrong: if (thd->variables.optimizer_use_condition_selectivity > 3 && table_share && !table_share->stats_cb.histograms_are_read) { (void) read_histograms_for_table(thd, tl->table, stat_tables); table_share->stats_cb.histograms_are_read= TRUE; } if (table_share->stats_cb.stats_is_read) tl->table->histograms_are_read= TRUE; Shouldn't second if check histograms_are_read instead?
            bar Alexander Barkov added a comment -

            The problem was found while working on MDEV-18899

            bar Alexander Barkov added a comment - The problem was found while working on MDEV-18899
            svoj Sergey Vojtovich added a comment -

            igor, please review top 11 patches at https://github.com/MariaDB/server/commits/bb-10.5-svoj-MDEV-19061

            svoj Sergey Vojtovich added a comment - igor , please review top 11 patches at https://github.com/MariaDB/server/commits/bb-10.5-svoj-MDEV-19061
            svoj Sergey Vojtovich added a comment -

            Patches backported to 10.1, also removed some cleanups.

            svoj Sergey Vojtovich added a comment - Patches backported to 10.1, also removed some cleanups.
            igor Igor Babaev added a comment -

            Ok to push into 10.1

            igor Igor Babaev added a comment - Ok to push into 10.1

            People

              svoj Sergey Vojtovich
              varun Varun Gupta (Inactive)
              Votes:
              1 Vote for this issue
              Watchers:
              8 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.