Index Cleanup - Job #3133
Index Cleanup - Job #3133
Conversation
| public async findInAppsForDigest(organizationId: string, transactionId: string, subscriberId: string) { | ||
| return await this.find({ | ||
| _organizationId: organizationId, | ||
| type: ChannelTypeEnum.IN_APP, | ||
| _subscriberId: subscriberId, | ||
| transactionId, | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Removed because this query is not in use
There was a problem hiding this comment.
An initial proposal I still want to overview again and see if we can make it fewer indexes.
There was a problem hiding this comment.
Left the notes so ill have a ref on the desition making will update them once ill finish.
| /* | ||
| * This index was initially created to optimize: | ||
| * apps/api/src/app/events/usecases/add-job/add-delay-job.usecase.ts | ||
| */ |
There was a problem hiding this comment.
Leaving the explanations for the different index optimisations are very helpful. 🙌🏻
There was a problem hiding this comment.
I think that even better would be to provide here some query examples? Because this comment is already not accurate as @LetItRock moved this file :P
Having some query examples here stay accurate even when code moves.
There was a problem hiding this comment.
Good point added the original query and the function name as a context
| /* | ||
| * This index was initially created to optimize: | ||
| * apps/api/src/app/events/usecases/add-job/add-delay-job.usecase.ts | ||
| */ |
There was a problem hiding this comment.
I think that even better would be to provide here some query examples? Because this comment is already not accurate as @LetItRock moved this file :P
Having some query examples here stay accurate even when code moves.
| transactionId: 1, | ||
| _subscriberId: 1, |
There was a problem hiding this comment.
Because of the cardinality of those 2 fields together I feel like we don't need all the other properties here of _templateId and _environmentId unless this could become a covered query. So having transactionId, _subscriberId, status should be enough in my opinion
| transactionId: 1, | ||
| _subscriberId: 1, |
There was a problem hiding this comment.
Following the top comment regarding the above query, I think this can be omitted?
There was a problem hiding this comment.
I agree, the next index below is a copy of this one.
From our discussion with mongo the order of the fields by the esr rule is the most important. Mongo should be smart enough to use the below index if this one is removed.
| _templateId: 1, | ||
| _subscriberId: 1, | ||
| _templateId: 1, |
There was a problem hiding this comment.
switched _subscriberId with _templateId because it has a higher cardinality
|
what do you think of changing to jobSchema.index({ That will mean that we could delete it because we already have: jobSchema.index({ At the moment they look really similar. |
| * ...(digestKey && { [`payload.${digestKey}`]: digestValue }), | ||
| * } | ||
| */ | ||
| jobSchema.index({ |
There was a problem hiding this comment.
I think this index may be "overrided" by the first query.
When you run this query, can you run https://www.mongodb.com/docs/manual/reference/method/cursor.explain/ to see which index the above query is running?
|
@djabarovgeorge we also need to add a single key index to support _notificationId queries, this is used by the activity feed populate from mongoose |
What change does this PR introduce?
Why? (Context)
We want to remove as many inefficient indexs and queries as possible.
Why was this change needed?
Stop using single indexes on the mongoose key implementation
Add compound indexes for common queries and patterns using MongoDB index best practices
Remove unused indexes from MongoDB atlas after implementing the new compound indexes
Other information (Screenshots)