-
Notifications
You must be signed in to change notification settings - Fork 218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SKYEDEN-3234 detect unused topics #1922
base: master
Are you sure you want to change the base?
Conversation
questras
commented
Nov 12, 2024
•
edited
Loading
edited
- Add a cron job to detect inactive topics and notify them
- notification thresholds, cron etc are configured via properties
- Notifier implementation is not provided, but the feature can work without it
- the job runs on one instance of hermes management only, based on provided ZK datacenter for leadership election (in properties)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, really clean solution and tests
public record InactiveTopic( | ||
@JsonProperty String qualifiedTopicName, | ||
@JsonProperty long lastPublishedMessageTimestampMs, | ||
@JsonProperty List<Long> notificationTimestampsMs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if it is necessary to keep all notification history as it can grow indefinitely. Perhaps we can persist only previous notification time? what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was my first thought but I decided to keep a list as it also tells you how many times you've already notified the topic, which might be useful.
What do you think about providing a configurable limit (via properties) of the list size? this way you still keep the history but it doesn't grow indefinitely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can go with bounded list