Bug#35952353 SELECT COUNT(*) degraded performance on 8.0 compared to 5.7

Symptoms
----------
The SELECT COUNT(*) query that uses secondary index for scan, takes more
than 6X with 8.0 compared to 5.7. Performance of the query that use the
clustered index for scan is at par with 5.7.

Background
----------
In 8.0.14, we introduced parallel scan of subtrees of a clustered index
through 'WL#11720 InnoDB: Parallel read of index'. This feature was
further improved through 'WL#12978 InnoDB:Fix imbalance during parallel
scan' in 8.0.17.
The idea of the second worklog was to utilize the parallel read threads
which were introduced through first worklog.
Second worklog also made another intentional change(not documented!)
that is, secondary index scan in InnoDB started using the parallel scan
of clustered index. In other words, even if Optimizer hints InnoDB to
use secondary index for scan, InnoDB ignores that, and uses clustered
index for scan. This observation was reported in the following bugs:

'Bug#31791868 INDEX HINT DOES NOT AFFECT COUNT(*) EXECUTION'
'Bug#35981123 SELECT COUNT is slow with O_DIRECT'

The second worklog enabled the clustered index (parallel) scan even for
secondary index scan for the following two reasons :
- Post WL#12978, performance of clustered index scan was much improved.
- Cost of scanning the secondary index is much higher to support the
  MVCC semantics i.e. read-committed, repeatable-read isolation levels.
  That is because before reporting a record from a secondary index,
  InnoDB must ensure it is visible to the query's read-view. That might
  require a dive into clustered index to consult corresponding row's
  undo chain. This can be avoided in case InnoDB sees that a secondary
  index record was not modified since read-view's creation, but
  unfortunately this information is not available per-row, only
  per-page. So, if any of the records on the page was modified since
  read-view's creation, the heuristic will not help, and dives to
  clustered index will be needed for all records from this secondary
  index page.

The second reason is main argument behind this change which can be
easily reproduced by modifying a few records in the table and then
executing the query in a session that has read_view created before
the modification, for example by having opened transaction on
isolation level of Repeatable Read before the modifications.
In my experiment, I found with secondary index scan(or with this fix),
query performed 30X worse than the query performed without a read view.

Thus it is really a trade off between using the clustered scan that is
better in worst case vs using the secondary index as Optimizer hints
which is better in most of the cases.

Fix
----
- Do not break the contract with Optimizer(Bug#31791868). User now
  (again) has an option to provide index hint to the query, to force
  using clustered index.

- We must improve Optimizer's heuristic so that it can hints the
  correct index to use. Created 'Bug#36297856 Query performance
  dropped by 30X with secondary index scan due to MVCC checks',
  for Optimizer team to improve the heuristic in Optimizer.

- Reverted part of the WL#12978 changes that forced clustered index
  (parallel) scan for secondary index scan.

Change-Id: Icb081d076cb6875e7b973e68ff6cfcce8ae3f82a

Author: RahulSisondia

mysql-test/r/select_count.result

@@ -52,10 +52,10 @@ Handler_discover	0
 Handler_external_lock	2
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	0
+Handler_read_first	1
+Handler_read_key	1
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0
@@ -100,10 +100,10 @@ Handler_discover	0
 Handler_external_lock	4
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	0
+Handler_read_first	1
+Handler_read_key	1
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0
@@ -148,10 +148,10 @@ Handler_discover	0
 Handler_external_lock	2
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	1
+Handler_read_first	1
+Handler_read_key	2
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0
@@ -225,10 +225,10 @@ Handler_discover	0
 Handler_external_lock	2
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	0
+Handler_read_first	1
+Handler_read_key	1
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0
@@ -279,10 +279,10 @@ Handler_discover	0
 Handler_external_lock	2
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	0
+Handler_read_first	1
+Handler_read_key	1
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0
@@ -404,10 +404,10 @@ Handler_discover	0
 Handler_external_lock	4
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	0
+Handler_read_first	1
+Handler_read_key	1
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0
@@ -452,10 +452,10 @@ Handler_discover	0
 Handler_external_lock	4
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	0
+Handler_read_first	1
+Handler_read_key	1
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0
@@ -487,10 +487,10 @@ Handler_discover	0
 Handler_external_lock	2
 Handler_mrr_init	0
 Handler_prepare	0
-Handler_read_first	0
-Handler_read_key	0
+Handler_read_first	1
+Handler_read_key	1
 Handler_read_last	0
-Handler_read_next	0
+Handler_read_next	3
 Handler_read_prev	0
 Handler_read_rnd	0
 Handler_read_rnd_next	0

storage/innobase/handler/ha_innodb.h

@@ -253,11 +253,6 @@ class ha_innobase : public handler {
 
   int records(ha_rows *num_rows) override;
 
-  int records_from_index(ha_rows *num_rows, uint) override {
-    /* Force use of cluster index until we implement sec index parallel scan. */
-    return ha_innobase::records(num_rows);
-  }
-
   ha_rows records_in_range(uint inx, key_range *min_key,
                            key_range *max_key) override;
 

storage/innobase/handler/ha_innopart.h

@@ -1114,11 +1114,6 @@ class ha_innopart : public ha_innobase,
 
   int records(ha_rows *num_rows) override;
 
-  int records_from_index(ha_rows *num_rows, uint) override {
-    /* Force use of cluster index until we implement sec index parallel scan. */
-    return ha_innopart::records(num_rows);
-  }
-
   int index_next(uchar *record) override {
     return (Partition_helper::ph_index_next(record));
   }