Initial setting of TTL index #3108
Conversation
| ); | ||
|
|
||
| messageSchema.index({ expireAt: 1 }, { expires: '1h' }); |
There was a problem hiding this comment.
1 hour after its expiration date - to allow buffer between archiving and its actual deletion. When we get to the archive will rethink that time amount.
There was a problem hiding this comment.
@ainouzgali I dont think this implementation is correct. By what the docs say this will ignore our actually ttl we set in the app and set every document to 1hour after creation.
https://www.mongodb.com/docs/manual/tutorial/expire-data/#expire-documents-at-a-specific-clock-time
There was a problem hiding this comment.
It deletes by the expireAt field, not the 1h. But it does ignore the expire. So it does not work right. Will try a different option
There was a problem hiding this comment.
The key thing for us is that the ttl should be set by the app not the db.
There was a problem hiding this comment.
Never mind @Cliftonz , It does work as expected - deletes by the expire date we set and then waits the amount waited in the expires field.
ainouzgali
changed the base branch from
next
to
nv-1871-introduce-a-ttl-index-to-large
| @@ -81,7 +82,27 @@ export class BaseRepository<T_DBModel, T_MappedEntity, T_Enforcement = object> { | |||
| } | |||
| } | |||
|
|
|||
| private calcExpireDate(modelName: string, data: FilterQuery<T_DBModel> & T_Enforcement) { | |||
| const now: number = Date.now(); | |||
| }, | ||
| schemaOptions | ||
| ); | ||
|
|
||
| notificationSchema.index({ expireAt: 1 }, { expires: '1h' }); |
There was a problem hiding this comment.
@ainouzgali let's do this 48 hours? just in case
|
@ainouzgali can we please have tests added to this aswell? Or is it part of another PR? |
| expireAt = new Date(email?.expireAt as string); | ||
| createdAt = new Date(email?.createdAt as string); | ||
|
|
||
| subExpireMonths = subMonths(expireAt, 1); | ||
| diff = differenceInMilliseconds(subExpireMonths, createdAt); | ||
|
|
||
| expect(diff).to.approximately(0, 1); |
There was a problem hiding this comment.
Is it possible to create constants somehwere in the top of the case so we can easily understand the intended behaviour? For example:
const IN_APP_MESSAGE_EXPIRE_MONTHS = 6;
const MESSAGE_EXPIRE_MONTHS = 1;
Right now need to manually do the logic to understand that the second param of the subMonths function is for this value.
Some more read about it: https://www.pluralsight.com/tech-blog/avoiding-magic-numbers/ :)
| async create(data: FilterQuery<T_DBModel> & T_Enforcement): Promise<T_MappedEntity> { | ||
| const expireAt = this.calcExpireDate(this.MongooseModel.modelName, data); | ||
| if (expireAt) { | ||
| data = { ...data, expireAt }; |
There was a problem hiding this comment.
| data = { ...data, expireAt }; | |
| data.expireAt = expireAt; |
Is there a particular reason for the ...destructuring?
I think it is not needed as we just can assign the needed property and save some memory :)
There was a problem hiding this comment.
I believe I tried that and had a typescript error. I will take another look
ainouzgali
merged commit 0985f0f
into
nv-1871-introduce-a-ttl-index-to-large
What change does this PR introduce?
WIP : an initial version of setting a ttl index for Message and Notification schemes.
I will of course, improve the function of expire date calculation- I first want to progress on how we will handle a notification/job that is delayed/ not completed on the time of its creation. And therefore its expiration date should take that into account.
Why was this change needed?
Other information (Screenshots)