openedx-unsupported / edx-analytics-pipeline

GNU Affero General Public License v3.0
91 stars 116 forks source link

add a combined index('username', 'entity_type') on module_engagement table #808

Closed muhammad-ammar closed 4 years ago

muhammad-ammar commented 4 years ago

Description: Add a combined index on module_engagement table. Index will created manually, we have opened a DE Support ticket for that.

JIRA: ENT-2548

Analytics Pipeline Pull Request

Make sure that the following steps are done before merging:

codecov[bot] commented 4 years ago

Codecov Report

Merging #808 into master will not change coverage by %. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #808   +/-   ##
=======================================
  Coverage   74.37%   74.37%           
=======================================
  Files         211      211           
  Lines       24399    24399           
=======================================
  Hits        18146    18146           
  Misses       6253     6253           
Impacted Files Coverage Δ
edx/analytics/tasks/insights/module_engagement.py 79.13% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f9e52fc...fcbf79b. Read the comment docs.

brianhw commented 4 years ago

I'm going to suggest that Hassan approve this after he applies the index to the table in production. Keep them in sync....

muhammad-ammar commented 4 years ago

@brianhw @HassanJaveed84 I have just verified on read replica and composite index is now added.

mysql> show indexes from module_engagement;
+-------------------+------------+----------------------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+
| Table             | Non_unique | Key_name             | Seq_in_index | Column_name | Collation | Cardinality | Sub_part | Packed | Null | Index_type | Comment | Index_comment |
+-------------------+------------+----------------------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+
| module_engagement |          0 | PRIMARY              |            1 | id          | A         |   975580605 |     NULL | NULL   |      | BTREE      |         |               |
| module_engagement |          1 | course_id            |            1 | course_id   | A         |    12349121 |     NULL | NULL   |      | BTREE      |         |               |
| module_engagement |          1 | course_id            |            2 | username    | A         |    57387094 |     NULL | NULL   |      | BTREE      |         |               |
| module_engagement |          1 | course_id            |            3 | date        | A         |   325193535 |     NULL | NULL   |      | BTREE      |         |               |
| module_engagement |          1 | date                 |            1 | date        | A         |      421235 |     NULL | NULL   |      | BTREE      |         |               |
| module_engagement |          1 | username_entity_type |            1 | username    | A         |    30486893 |     NULL | NULL   |      | BTREE      |         |               |
| module_engagement |          1 | username_entity_type |            2 | entity_type | A         |    57387094 |     NULL | NULL   |      | BTREE      |         |               |
+-------------------+------------+----------------------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+
7 rows in set (0.00 sec)

I think we are good to merge this PR.