feat: introduce ttl index #3172
Conversation
NV-1871 Introduce a TTL index to large collections
Why? (Context)We want to introduce TTL to so that certain data is only kept for a certain a mount of time. Definition of Done
|
…tl-index-to-large # Conflicts: # libs/dal/src/repositories/message/message.schema.ts
| @@ -255,6 +258,77 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { | |||
| expect(email.channel).to.equal(ChannelTypeEnum.EMAIL); | |||
| }); | |||
|
|
|||
| it('should correctly set expiration date (TTL) for notification and messages', async function () { | |||
There was a problem hiding this comment.
Would maybe split this to 3 test cases for each channel type it might be more easier to read. Wdyt?
There was a problem hiding this comment.
I think I will add a new test suite for this on Thursday, to also check with overrides and scheduled delay and multiple delayed actions in one workflow @scopsy
| }, | ||
| schemaOptions | ||
| ); | ||
|
|
||
| notificationSchema.index({ expireAt: 1 }, { expires: '48h' }); |
There was a problem hiding this comment.
@ainouzgali speaking with @Cliftonz it seems we only need to add the "Expires" path for self hosted right? the online archive with take care for it for us?
There was a problem hiding this comment.
I suspect it will not do any harm not to add the check. As the ttl deletion will only run 48 hours after the online archive. If its already deleted, then ttl won't run.
But I would like to make sure when we have the integration with the online archive, and I wouldn't mind adding it @Cliftonz @scopsy
There was a problem hiding this comment.
@ainouzgali I do not think we should have both. Both act as a ttl and if the backlog of the archival process gets two log then we could lose data.
I think we should do something like the following:
if Novu_Managed && Enable_TTL
enable ttl index
else
only create index on expireAt for online archive
This way users who want to keep everything can and users who dont can have them deleted after the standard amount of time.
We can make community issues to add configurations for the ttl for each entity we are supporting.
There was a problem hiding this comment.
So we have two things here:
- managed should not have 'expire'(ttl) - only by archive
- self hosted should have enable_ttl field -> meaning they will have the option to either have our settings for the ttl or to not have it at all
Have I got that right?
There was a problem hiding this comment.
So basically for Managed Service:
messageSchema.index({ expireAt: 1 });
And for Self hosted:
messageSchema.index({ expireAt: 1 }, { expires: '48h' });
What change does this PR introduce?
We want to introduce TTL to so that certain data is only kept for a certain a mount of time.
Why was this change needed?
Other information (Screenshots)