You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains 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
When the volume.Spec.NodeID is different than the node ID of the backup VA ticket, we should not initiate a backup as the volume is going to detach soon
This PR is going to replace the approach at the PR #2627. After discussed with @ejweber and @james-munson , we think that this approach is better because:
The other approach only touches the logic for the VA ticket of RWO non-migratible volume. It still leaves the logic for the RWX and mitigrable volume the same as we cannot compare the volume.Spec.NodeID with the node ID of these VA tickets. They are most likely to be different
It is fundamentally still correct that when the volume start detaching (but not finish detaching yet) the VA ticket can set to satisfied because at this exact moment the volume is still attached. VA controller can set reset the VA ticket to satisfied:false when the volume finish detaching. It is the responsibility of the client (backup controller in this case) to check and make sure that the volume.Spec.NodeID is still the desired one
When the volume.Spec.NodeID is different than the node ID of the backup
VA ticket, we should not initiate a backup as the volume is going to
detach soon
longhorn-7937
Signed-off-by: Phan Le <phan.le@suse.com>
The fix is general, so it should be expected to happen in 1.6 and 1.5 after VA was introduced. Do you know why this issue can reproduce after 1.5.4-RC2? @PhanLe1010@c3y1huang
Hi @innobead my current theory is that we change the code flow somehow and make it longer between volume.Spec.NodeID being cleanup and the time that volume starts detaching by setting the engine.spec.desirestate to stop. But that is just an idea and I couldn't find proof from the git diff of recent commits. I believe @c3y1huang and @mantissahz already searched the commit history too. We couldn't pin down the exact commit that made the race become more visible
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 the volume.Spec.NodeID is different than the node ID of the backup VA ticket, we should not initiate a backup as the volume is going to detach soon
longhorn/longhorn#7937
This PR is going to replace the approach at the PR #2627. After discussed with @ejweber and @james-munson , we think that this approach is better because: