to clarify

KEY `commitID` (`commitID`,`isDirect`,`changeType`)

is the new index.

Ah, nice!

That key seems like it's probably not great for the OR changeType part of the query. If you still have the table handy and playing with it doesn't take an unreasonably huge amount of time, is this better?

• Add KEY (commitID, isDirect).
• Add KEY (commitID, changeType).

Then run this variant of the query:

SELECT commitID, pathID FROM `repository_pathchange` WHERE commitID IN (...) AND isDirect = 1
UNION ALL
SELECT commitID, pathID FROM `repository_pathchange` WHERE commitID IN (...) AND changeType = 10;

I think that should go fast, and maybe have a better query plan than the triple (commitID, isDirect, changeType) key?

That variant of the query is slow as well.

Hmm, weird. Can you show me the EXPLAIN SELECT ... for that?

id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
1	PRIMARY	repository_pathchange	index	PRIMARY,cidid,cidct	cidid	5	NULL	303320447	Using where; Using index
2	UNION	repository_pathchange	index	PRIMARY,cidid,cidct	cidct	8	NULL	303320447	Using where; Using index
NULL	UNION RESULT	<union1,2>	ALL	NULL	NULL	NULL	NULL	NULL	Using temporary

is the explanation of the query

id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
1	PRIMARY	repository_pathchange	index	PRIMARY,cidid,cidct	cidid	5	NULL	303320519	100.00	Using where; Using index
2	UNION	repository_pathchange	index	PRIMARY,cidid,cidct	cidct	8	NULL	303320519	100.00	Using where; Using index
NULL	UNION RESULT	<union1,2>	ALL	NULL	NULL	NULL	NULL	NULL	NULL	Using temporary

KEY `cidid` (`commitID`,`isDirect`),
KEY `cidct` (`commitID`,`changeType`)

are the keys created

Hrrm, let me see if I can reproduce anything like that, that's totally bizarre to me.

to clarify: I removed the key I originally added

KEY `commitID` (`commitID`,`isDirect`,`changeType`)

was I not supposed to do that?

That's fine -- it looks like it's picking up the keys, I'm just not sure why it's then examining 303 million rows when it has the keys available.

It also seems like the table primary key should get us almost all the way there regardless of these other keys.

We only have ~300K rows locally (so the table is 1000x smaller) but I get reasonable query plans even with the unmodified query, using the primary key:

mysql> explain SELECT commitID, pathID FROM repository_pathchange WHERE commitID IN (18102, 18101, 18100, 18099, 18098, 18097, 18096, 18095, 18094, 18093, 18092, 18091, 18090, 18089, 18088, 18087, 18086, 18085, 18084, 18083, 18082, 18081, 18080, 18079, 18078, 18077, 18076, 18075, 18074, 18073, 18072, 18071, 18070, 18069, 18068, 18067, 18066, 18065, 18064, 18063, 18062, 18061, 18060, 18059, 18058, 18057, 18056, 18055, 18054, 18053, 18052, 18051, 18050, 18049, 18048, 18047, 18046, 18045, 18044, 18043, 18042, 18041, 18040, 18039, 18038, 18037, 18036, 18035, 18034, 18033, 18032, 18031, 18030, 18029, 18028, 18027, 18026, 18025, 18024, 18023, 18022, 18021, 18020, 18019, 18018, 18017, 18016, 18015, 18014, 18013, 18012, 18011, 18010, 18009, 18008, 18007, 18006, 18005, 18004, 18003) AND (isDirect = 1 OR changeType = 10);
+----+-------------+-----------------------+-------+---------------+---------+---------+------+------+-------------+
| id | select_type | table                 | type  | possible_keys | key     | key_len | ref  | rows | Extra       |
+----+-------------+-----------------------+-------+---------------+---------+---------+------+------+-------------+
|  1 | SIMPLE      | repository_pathchange | range | PRIMARY       | PRIMARY | 4       | NULL | 1784 | Using where |
+----+-------------+-----------------------+-------+---------------+---------+---------+------+------+-------------+
1 row in set (0.00 sec)

Actual number of result rows is 1,761, so the examined row value is reasonable.

If you remove the (isDirect = 1 OR changeType = 10) clause, does the query return an unreasonably ridiculous number of rows?

I suppose the originally proposed key may be better than I think. I was imagining that it couldn't use the third part of the key (on changeType) but I suppose it actually can. I'm still confused by the UNION ALL behavior, but if the key's working on your dataset I think it's fine to add it -- it takes <1 second to add to our dataset, so even for large datasets it shouldn't be wildly unreasonable, even if we do find a better key later.

I'll leave this open for now since the behavior still feels a bit spooky to me, let me know how things go once you get the key in place? If they're smooth, we can chalk this up to be being bad at MySQL.

Hrm, I'm going to do some followup with our DBAs as well. The performance numbers above were copied from the DBA analysis; however, when I make the change in production, rather than in staging, some of the behavior does not directly replicate.

Hopefully we'll figure this out and be able to provide more detailed information.

Quick update here. We've found that adding FORCE INDEX (primary) results in 79.4ms in production even without the key_history index.

I'm still working with our DBAs to discover (a) the difference between our production and staging environments which cause the performance differences and (b) why the query optimizer isn't picking the primary key on its own.

Without the key_history index and without FORCE INDEX the result of EXPLAIN EXTENDED shows that primary is a candidate, but not selected.

Interesting (and weird). Thanks for the update -- and good luck figuring things out.

The discrepancy between staging + production is almost certainly caused by the size of the data.

You do not need to create a secondary index on changeId as that is the leftmost part of the primary key. The secondary index just points to the primary index where the data is stored.

The underlying cause of the slowness is because the stats were completely off in the production table (was showing cardnality of 18 for changeId).

https://bugs.mysql.com/bug.php?id=36513 is also related.

Should we get rid of the <commitID, isDirect, changeType> key, then?

We could possibly work around MySQL 36513 by doing something like this periodically (say, daily as part of GC):

• Set innodb_stats_sample_pages to a larger value than 8.
• Explicitly ANALYZE any tables which we know are prone to cardinality errors.

I'm not sure if that would help or not. If the alternative is something along the lines of a DBA doing this manually this might be an improvement, though.

Well, the docs suggest not doing that:

Because the statistics are automatically recalculated at various times other than on execution of ANALYZE TABLE, it does not make sense to increase the index sample size, run ANALYZE TABLE, then decrease sample size again. The more accurate statistics calculated by ANALYZE running with a high value of innodb_stats_sample_pages can be wiped away later.
https://dev.mysql.com/doc/refman/5.5/en/innodb-statistics-estimation.html

We could also consider suggesting users raise the default value of 8, but I'm not sure what the accuracy/performance tradeoff curve looks like.

In T10560#165431, @epriestley wrote:

Should we get rid of the <commitID, isDirect, changeType> key, then?

I think so. Sorry for the originally misleading claim.

We could possibly work around MySQL 36513 by doing something like this periodically (say, daily as part of GC):

• Explicitly ANALYZE any tables which we know are prone to cardinality errors.

I'm not sure if that would help or not. If the alternative is something along the lines of a DBA doing this manually this might be an improvement, though.

I think triggering ANALYZE as a part of T9554 is most reasonable here. In addition it may be helpful to periodically check table cardinality and run ANALYZE if it seems wildly off. One concern with running it daily is that depending on the version of MySQL it may cause a table lock to be taken.

I think so. Sorry for the originally misleading claim.Sorry for the originally misleading claim.

No problem, this stuff is easy to swap out and I hadn't seen the cardinality issue before so I'll have another idea of something to look for in the future.

We could also make bin/storage adjust just run ANALYZE on every table, since we know it's safe to take table locks during a schema change anyway. Let me see if that takes an unreasonably long amount of time for a moderately-sized install. I'm not sure if it'll really help, but it probably shouldn't hurt.

Here's a diff to make bin/storage adjust always run ANALYZE on every table after it makes other adjustments, but I'm not really sure if that's actually helpful or not (e.g., from the documentation, it sounds like the good analysis can be immediately overwritten by a bad one in some cases?):

https://secure.phabricator.com/differential/diff/37323/

It only adds 1-2 seconds to the cost of storage adjust on my local dataset so the cost is very small, but I'd like a better understanding of exactly when ANALYZE can fix a problem before bringing it upstream.

We'll try that in our next upgrade. Thanks for the help!

[ we consider this issue resolved, but I think you had some plans for adding additional setup checks so I won't change the status ]