Skip to content
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

pass device path on publish from controller #268

Merged
merged 1 commit into from
May 16, 2024
Merged

Conversation

WanzenBug
Copy link
Member

When the node plugin receives a publish (== mount) call, it needs to know what device needs to be mounted. Up to now, this was done by an API request to LINSTOR. With the introduction of more aggressive caching, this often meant the request would end in a 404 since the cache did not know about the volume at all.

To fix this, we move the retrieval of the device path out of the node plugin entirely. We can simply pass the device path from the ControllerPublish() call down using the publish context. There, the cache is not an issue since for the volume to be created, some kind of modification is needed, which will always result in cache invalidation.

We also remove the request for the device path from the NodeExpand() call: we have to look at the local mount table so we get the device name from there.

With this change, the node plugin should no longer need to request LINSTOR API outside the node/storage pool information, which is not affected by this caching issue.

@WanzenBug WanzenBug force-pushed the fix-caching-issues branch 2 times, most recently from 24254af to bc4ab74 Compare May 16, 2024 08:42
When the node plugin receives a publish (== mount) call, it needs to know
what device needs to be mounted. Up to now, this was done by an API request
to LINSTOR. With the introduction of more aggressive caching, this often
meant the request would end in a 404 if multiple volumes where mounted in
short succession.

To fix this, we move the retrieval of the device path out of the node plugin
entirely. We can simply pass the device path from the ControllerPublish() call
down using the publish context.

We also remove the request for the device path from the NodeExpand() call:
we have to look at the local mount table so we get the device name from there.

With this change, the node plugin should no longer need to request LINSTOR API
outside the node/storage pool information, which is not affected by this caching
issue.

Signed-off-by: Moritz Wanzenböck <[email protected]>
@WanzenBug WanzenBug force-pushed the fix-caching-issues branch from bc4ab74 to 90a8469 Compare May 16, 2024 08:49
@WanzenBug WanzenBug marked this pull request as ready for review May 16, 2024 10:33
@WanzenBug WanzenBug requested a review from rck May 16, 2024 10:35
Copy link
Member

@rck rck left a comment

Choose a reason for hiding this comment

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

looks plausible

@rck rck merged commit 08a17ce into master May 16, 2024
5 checks passed
@rck rck deleted the fix-caching-issues branch May 16, 2024 10:53
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