Skip to content

feat(linstorvolumemanager): cache controller uri in a file #83

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

Open
wants to merge 17 commits into
base: 3.2.12-8.3
Choose a base branch
from

Conversation

Millefeuille42
Copy link

No description provided.

@Millefeuille42 Millefeuille42 self-assigned this Mar 5, 2025
@Wescoeur Wescoeur changed the base branch from 3.2.3-8.3 to 3.2.12-8.3 March 20, 2025 14:33
@Millefeuille42 Millefeuille42 force-pushed the mlr-533-cache-controller-uri-in-a-file branch 2 times, most recently from 958caec to 42b42bc Compare March 20, 2025 14:53
Comment on lines 238 to 244
address = uri.removeprefix("linstor://")
session = util.timeout_call(10, util.get_localAPI_session)
for host_ref, host_record in session.xenapi.host.get_all():
if host_record.get('address', '') != address:
continue
return util.strtobool(
session.xenapi.host.call_plugin(host_ref, PLUGIN, PLUGIN_CMD, {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that's necessary here. We can instead do a refactoring of the codebase:

  • We will assume that the cache is valid 99% of the time.
  • We can directly attempt to create a LINSTOR instance from the URI without checking anything using a plugin. This is the initial idea.
  • I think we can implement a new static function on LinstorVolumeManager that is used to create an instance of the class using the cached value directly without any checks. If it fails, we rebuild the local cache and try again. This allows us to improve several smapi functions.
  • There are a few edge cases left with this idea:
    • In some places we use the URI to create the journaler (linstor.KV) and to create a linstor object. We could try to use the cached URI without checks again and add a try/catch directly on these specific cases. In case of connection failure, we explicitly request the cache update.
    • I see one last edge case concerning the creation of a linstor instance using get_ips_from_xha_config_file, we could directly use the function to get the cached URI. In the worst can we can again fallback on the xha config file.

Copy link
Author

@Millefeuille42 Millefeuille42 Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge cases are almost done. I pushed modification of the added functions 042014f and created the static method you suggested 0364eed. Could you please take a look at the current state of those functions to validate them before I start the refactor ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good from my POV. Feel free to continue when you have time. :)

@Millefeuille42 Millefeuille42 force-pushed the mlr-533-cache-controller-uri-in-a-file branch from 42b42bc to 065e30c Compare April 28, 2025 09:31
@Millefeuille42 Millefeuille42 requested a review from Wescoeur May 16, 2025 14:40
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
@Millefeuille42 Millefeuille42 force-pushed the mlr-533-cache-controller-uri-in-a-file branch from c315328 to 9760980 Compare June 17, 2025 09:30
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.

2 participants