Split subscription discovery to a 900 sec interval

trento-project / trento

An open cloud-native web console improving on the work day of SAP Applications administrators.

https://www.trento-project.io

Apache License 2.0

33 stars 26 forks source link

Split subscription discovery to a 900 sec interval #876

Closed rtorrero closed 2 years ago

rtorrero commented 2 years ago

This PR's main goal is to reduce the possible load we are hitting our SCC service with. It attempts this by:

Splitting the subscription discovery from the rest of the discoveries so that we can set a separate interval
Increasing the default interval time to a tick per each 15 minutes.

In addition to this, we have added a hidden parameter in cobra to allow quickly adjusting this time interval. Feedback if this hidden param is a good idea or not is appreciated!

rtorrero commented 2 years ago

I see the 900s (15m) discovery period for subscription info. For me it is good. Maybe update the PR title to reflect this number?

woops! you are right @abravosuse, sorry!

dottorblaster commented 2 years ago

Same comment as @arbulu89, but for the current iteration is fine for me. @fabriziosestito @nelsonkopliku what do you think?

abravosuse commented 2 years ago

Following up on @arbulu89 idea, maybe we could set minimum values for certain individual discoveries? For instance, for the subscription info discovery..

fabriziosestito commented 2 years ago

Same comment as @arbulu89, but for the current iteration is fine for me. @fabriziosestito @nelsonkopliku what do you think?

I also agree it in making it configurable for all the loops. What I don't really dig about the proposed approach is that we have a different (duplicated) loop for the subscription. I would rather have a start ticker function where we can pass the discovery time, so we can dry up the code as well.

rtorrero commented 2 years ago

Following up on @arbulu89 idea, maybe we could set minimum values for certain individual discoveries? For instance, for the subscription info discovery..

That sounds like a good idea for me; add some sanity checks preventing values that don't make much sense.

Same comment as @arbulu89, but for the current iteration is fine for me. @fabriziosestito @nelsonkopliku what do you think?

I also agree it in making it configurable for all the loops. What I don't really dig about the proposed approach is that we have a different (duplicated) loop for the subscription. I would rather have a start ticker function where we can pass the discovery time, so we can dry up the code as well.

This sounds good to me too, should we merge this meanwhile or go directly with the better approach?

rtorrero commented 2 years ago

Closing this as #881 includes this (plus a different approach on the discovery intervals in general)