[db-manager] Move rid cleanup cronjob to db-manager evict #1261
Conversation
| checkScdSubs = flags.Bool("scd_sub", true, "set this flag to true to check for expired SCD subscriptions") | ||
| checkRidISAs = flags.Bool("rid_isa", true, "set this flag to true to check for expired RID ISAs") | ||
| checkRidSubs = flags.Bool("rid_sub", true, "set this flag to true to check for expired RID subscriptions") | ||
| ttl = flags.Duration("ttl", time.Hour*24*112, "time-to-live duration used for determining SCD entries expiration, defaults to 2*56 days which should be a safe value in most cases") |
There was a problem hiding this comment.
Question about ttl. Should we:
- Use a common value ?
- Use a separate parameter ?
- Use a fixed value of 30m, hardcoded in rid store (like previous cron)
Currently version 3 is implemented
There was a problem hiding this comment.
I may expect different jobs running at different frequency for each services. So I would suggest to keep ttl and apply it to all 4 resources. If a different value is required, several job can be configured. You may want to validate this assumption on the issue.
| if err = ridStore.Transact(ctx, ridAction); err != nil { | ||
| return fmt.Errorf("failed to execute RID transaction: %w", err) | ||
| } |
There was a problem hiding this comment.
we might want to completely skip the action when there's not flag associated to the store
There was a problem hiding this comment.
Hum, not sure I agree: we left the function itself to decide what to run, no need to copy-paste and maintain the logic at two location no? It has no performance impact?
| checkScdSubs = flags.Bool("scd_sub", true, "set this flag to true to check for expired SCD subscriptions") | ||
| checkRidISAs = flags.Bool("rid_isa", true, "set this flag to true to check for expired RID ISAs") | ||
| checkRidSubs = flags.Bool("rid_sub", true, "set this flag to true to check for expired RID subscriptions") | ||
| ttl = flags.Duration("ttl", time.Hour*24*112, "time-to-live duration used for determining SCD entries expiration, defaults to 2*56 days which should be a safe value in most cases") |
There was a problem hiding this comment.
I may expect different jobs running at different frequency for each services. So I would suggest to keep ttl and apply it to all 4 resources. If a different value is required, several job can be configured. You may want to validate this assumption on the issue.
| logExpiredEntity("SCD subscription", sub.ID, threshold, *deleteExpired, sub.EndTime != nil) | ||
| } | ||
| for _, isa := range expiredISAs { | ||
| logExpiredEntity("ISA", isa.ID, time.Now().Add(-time.Duration(ridc.ExpiredDurationInMin)*time.Minute), *deleteExpired, isa.EndTime != nil) |
There was a problem hiding this comment.
Assuming the deletion in the action above takes a lot of time, the usage of time.Now() seems inappropriate.
| logExpiredEntity("ISA", isa.ID, time.Now().Add(-time.Duration(ridc.ExpiredDurationInMin)*time.Minute), *deleteExpired, isa.EndTime != nil) | ||
| } | ||
| for _, sub := range ridExpiredSub { | ||
| logExpiredEntity("RID subscription", sub.ID, time.Now().Add(-time.Duration(ridc.ExpiredDurationInMin)*time.Minute), *deleteExpired, sub.EndTime != nil) |
Base version for implementation of #1250 , with some open questions :)
Doesn't include infrastructure changes.