api: Avoid race-condition in volume-attach timeout handling#593
Open
leust wants to merge 1 commit intostable/2023.2-m3from
Open
api: Avoid race-condition in volume-attach timeout handling#593leust wants to merge 1 commit intostable/2023.2-m3from
leust wants to merge 1 commit intostable/2023.2-m3from
Conversation
joker-at-work
left a comment
joker-at-work
left a comment
There was a problem hiding this comment.
Looks good, just 2 minor things.
nova/tests/unit/compute/test_api.py
Outdated
Comment on lines
+594
to
+596
| self.assertRaises(oslo_exceptions.MessagingTimeout, | ||
| self.compute_api.attach_volume, | ||
| self.context, instance, volume['id']) |
nova/compute/api.py
Outdated
| objects.BlockDeviceMapping.get_by_volume_and_instance( | ||
| context, volume['id'], instance.uuid) | ||
| if bdm.attachment_id: | ||
| LOG.warning( |
There was a problem hiding this comment.
You sure this needs to be a warning? I think info would be enough - debug would be fine, too, imho.
Could we start the log-string on this same line like below for the LOG.debug after the bdm.destroy()?
When nova-api calls reserve_block_device_name RPC to nova-compute and the call times out, we try to clean up by deleting the BDM entry. However, during the timeout window a second attachment request for the same volume can come in, create a valid BDM, and progress to talking to Cinder. The original timed-out request then deletes this new valid BDM, leaving the volume in an inconsistent state. We fix this by checking if the BDM has attachment_id set before deleting it. The attachment_id field is only populated in _check_attach_and_reserve_volume(), which we only call after the reserve_block_device_name RPC succeeds. If attachment_id is set, we know the BDM belongs to a subsequent request that has already progressed past the RPC phase, so we should not delete it. Change-Id: I7ed649a5cab7f254690f329fac285128d8cd1c92
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When nova-api calls reserve_block_device_name RPC to nova-compute and the call times out, we try to clean up by deleting the BDM entry. However, during the timeout window a second attachment request for the same volume can come in, create a valid BDM, and progress to talking to Cinder. The original timed-out request then deletes this new valid BDM, leaving the volume in an inconsistent state.
We fix this by checking if the BDM has attachment_id set before deleting it. The attachment_id field is only populated in _check_attach_and_reserve_volume(), which we only call after the reserve_block_device_name RPC succeeds. If attachment_id is set, we know the BDM belongs to a subsequent request that has already progressed past the RPC phase, so we should not delete it.
Change-Id: I7ed649a5cab7f254690f329fac285128d8cd1c92