Skip to content

utilize ServiceInfo from database for collect-startup and serve-tasks #725

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wagnerd3
Copy link
Contributor

Checklist:

  • If this PR is about a plugin, I tested the plugin against an OpenStack cluster.
  • I updated the documentation to describe the semantical or interface changes I introduced.

Contents of this PR:

  • collect task falls back to database values in case of Liquid not being available on startup
  • non-collect tasks always use values from the database without cache, through the Cluster object
    • the DataMetricsReporter is the only user which caches the info for the duration of one assembly of the metrics
  • only collector-stuff accesses the LiquidConnections directly

This is not yet tested properly for all cases, because the datasources in qa-de-1 are partially down. Therefore opening as draft for now.

@coveralls
Copy link

coveralls commented May 26, 2025

Coverage Status

coverage: 78.479% (-0.6%) from 79.123%
when pulling ec0f32d on utilize_service_info_from_db2
into fcd2bfb on master.

@wagnerd3 wagnerd3 force-pushed the utilize_service_info_from_db2 branch 6 times, most recently from 78d73c6 to b62e775 Compare May 28, 2025 08:04
@wagnerd3 wagnerd3 force-pushed the utilize_service_info_from_db2 branch from b62e775 to 68a0a96 Compare May 28, 2025 12:45
@wagnerd3 wagnerd3 force-pushed the utilize_service_info_from_db2 branch from 68a0a96 to 02df4cd Compare May 28, 2025 13:54
@wagnerd3
Copy link
Contributor Author

wagnerd3 commented Jun 2, 2025

I have adressed the comments from @majewsky now. I initially started to pass through the errors of all the small helper functions everywhere, but quickly realized that by checking for the errors everywhere the code gets very messy. Additionally, because of the many specific methods to access the ServiceInfo from the DB, it was hard to keep track of double-access to the DB.

I did solve this a little bit different now: On Cluster level, the access is just possible to access all ServiceInfos as a whole - for all services or just for a single one. The utility functions get this object. This way, it is much easier to track where the object should be assembled. Sometimes, this means we do 1 unnecessary DB call for the rates, but this is still better than doing X calls for every single hasService etc.

I had to add this serviceInfos object to very many function signatures to pass it through, so I am not yet sure whether this is the final/ nicest solution. Better ideas are always welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants