Add container indexes #699
Conversation
These are focused on speeding up the container related screens. These are also focused on the current implementation of the associations and their use of deleted_on IS NULL. We probably want to circle back once our archived patterns change.
|
Checked commit kbrock@b050928 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
| # container projects page | ||
| add_index :container_routes, :container_project_id | ||
| add_index :container_services, :container_project_id | ||
| # didn't show much, since only had 1 record |
There was a problem hiding this comment.
This comment doesn't help future readers, so let's remove the comment. It's fine in the commit message and/or PR comments though)
| add_index :taggings, [:taggable_type, :taggable_id, :tag_id], :name => "index_taggings_on_type_id_id" | ||
| remove_index :taggings, :column => [:taggable_id, :taggable_type], :name => "index_taggings_on_taggable_id_and_taggable_type" | ||
|
|
||
| # container Services |
There was a problem hiding this comment.
consistent caps
| # container Services | |
| # container services |
| # NOTE: this gets index only scans, but has a larger index size | ||
| add_index :taggings, [:taggable_type, :taggable_id, :tag_id], :name => "index_taggings_on_type_id_id" | ||
| remove_index :taggings, :column => [:taggable_id, :taggable_type], :name => "index_taggings_on_taggable_id_and_taggable_type" |
There was a problem hiding this comment.
I don't understand this at all. Typically pattern for indexes is to put the highest cardinality first, unless there are queries where the lower cardinality one can stand alone. Also I don't understand adding tag_id
There was a problem hiding this comment.
Yes, this is looked up using taggable_type,taggable_id - the taggable_type is held constant and there are many taggable_id values.
Adding the tag_id to the index allows the tag_id to be pulled from the index, so it doesn't need to hit the data page to look up the tag_id.
We use tagging a lot, so I felt that skiping the data page would help us. But this does make the index larger, so maybe we don't want to go this route.
There was a problem hiding this comment.
I'm not against adding the tag_id - I'm questiong why taggable_type and taggable_id order were reversed - that's the part I'm questioning.
| # container nodes | ||
| add_index :container_conditions, [:container_entity_type, :container_entity_id, :name], :name => "index_container_conditions_on_cet_ceid_name" |
There was a problem hiding this comment.
The virtual attribute that this is targeting is:
(SELECT container_conditions.status
FROM container_conditions
WHERE container_conditions.name = 'Ready'
AND container_conditions.container_entity_id = container_nodes.id
AND container_conditions.container_entity_type = 'ContainerNode')
AS ready_condition_status,
When we have 2 indexes, one with id first and the other with type first, the analyzer prefers the one with type first. When I delete the type first index, it is still able to use the id first index with similar query statistics.
Having name, type, id or WHERE name = 'READY' would be faster, but this way, the index can also be used for all container_conditions, not just the ready ones.
There was a problem hiding this comment.
When we have 2 indexes, one with id first and the other with type first, the analyzer prefers the one with type first. When I delete the type first index, it is still able to use the id first index with similar query statistics.
I'm not sure that's an entirely accurate assessment. Or rather, statistically, if they are the same, it will just pick one, but that doesn't then imply that type,id is better than id,type.
Based on all my readings, higher cardinality should always be first because it eliminates more rows and allows the index to execute faster, because the row elimination happens on the first pass of the index. So id,type is nearly always preferable over type,id. The only time this shouldn't be true is if we have queries that don't provider the _id in the query at all.
Having
name, type, id...
Not my suggestion - I think it should be id, type, name
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
3 similar comments
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
These are focused on speeding up the container related screens. These are also focused on the current implementation of the associations and their use of deleted_on IS NULL.
We probably want to circle back once our archived patterns change.
These are even more noticeable after we add a few virtual attributes that exercise these relationships in a non-N+1 way.