[MDEV-19061] table_share used for reading statistical tables is not protected Created: 2019-03-27  Updated: 2023-07-31  Resolved: 2020-05-29

Status: Closed
Project: MariaDB Server
Component/s: Optimizer
Affects Version/s: 10.1, 10.2, 10.3, 10.4
Fix Version/s: 10.5.4, 10.1.46, 10.2.33, 10.3.24, 10.4.14

Type: Bug Priority: Major
Reporter: Varun Gupta (Inactive) Assignee: Sergey Vojtovich
Resolution: Fixed Votes: 1
Labels: None

Issue Links:
Relates
relates to MDEV-18899 Server crashes in Field::set_warning_... Closed
relates to MDEV-19407 Assertion `field->table->stats_is_rea... Closed
relates to MDEV-29693 ANALYZE TABLE still flushes table def... Closed

 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



 Comments   
Comment by Varun Gupta (Inactive) [ 2019-03-27 ]

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;

Comment by Sergey Vojtovich [ 2019-03-27 ]

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?

Comment by Alexander Barkov [ 2019-03-28 ]

The problem was found while working on MDEV-18899

Comment by Sergey Vojtovich [ 2020-04-21 ]

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

Comment by Sergey Vojtovich [ 2020-05-28 ]

Patches backported to 10.1, also removed some cleanups.

Comment by Igor Babaev [ 2020-05-29 ]

Ok to push into 10.1

Generated at Thu Feb 08 08:48:46 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.