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

Various fixes for idempotency, error messages and concurrency #1552

Merged
merged 6 commits into from
Nov 29, 2023

Conversation

tiagolobocastro
Copy link
Contributor

@tiagolobocastro tiagolobocastro commented Nov 28, 2023

fix(grpc/snapshot): snapshot grpc should use replica svc

We should not allow concurrent snapshot and replica operations as they might clash.
todo: allow per-resource locking
Furthermore it was observed that holding a bdev ptr whilst it's being deleted might
cause a crash, we need to determine if this was to do with it being held over an
async point, or a more general problem.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

ci(test/wipe/replicas): allow list and wipe replicas

System test is using wipe replicas, which can block list calls from the control-plane
and thus confusing it thinking the node is not responding.
Let's allow concurrent wipe with lists, since wiping doesn't destroy or modify
replica metadata anyway.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

fix(io-engine/client): fix cli args

Some args were not setup correctly since clap update.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

fix: bad error message

ChildNotFound error had an invalid error message.
Don't output NotFound messages for ptpl.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

fix(lvs/status): fixup missing bdev errors

Allows the control-plane to determine what the inner error was.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

Allows the control-plane to determine what the inner error was.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
ChildNotFound error had an invalid error message.
Don't output NotFound messages for ptpl.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Some args were not setup correctly since clap update.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
System test is using wipe replicas, which can block list calls from the control-plane
and thus confusing it thinking the node is not responding.
Let's allow concurrent wipe with lists, since wiping doesn't destroy or modify
replica metadata anyway.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro tiagolobocastro changed the title fix(lvs/status): convert missing bdev error Various fixes for idempotency, error messages and concurrency Nov 29, 2023
@tiagolobocastro
Copy link
Contributor Author

bors merge

bors-openebs-mayastor bot pushed a commit that referenced this pull request Nov 29, 2023
1552: Various fixes for idempotency, error messages and concurrency r=tiagolobocastro a=tiagolobocastro

    fix(grpc/snapshot): snapshot grpc should use replica svc
    
    We should not allow concurrent snapshot and replica operations as they might clash.
    todo: allow per-resource locking
    Furthermore it was observed that holding a bdev ptr whilst it's being deleted might
    cause a crash, we need to determine if this was to do with it being held over an
    async point, or a more general problem.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    ci(test/wipe/replicas): allow list and wipe replicas
    
    System test is using wipe replicas, which can block list calls from the control-plane
    and thus confusing it thinking the node is not responding.
    Let's allow concurrent wipe with lists, since wiping doesn't destroy or modify
    replica metadata anyway.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    fix(io-engine/client): fix cli args
    
    Some args were not setup correctly since clap update.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    fix: bad error message
    
    ChildNotFound error had an invalid error message.
    Don't output NotFound messages for ptpl.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    fix(lvs/status): fixup missing bdev errors
    
    Allows the control-plane to determine what the inner error was.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

Co-authored-by: Tiago Castro <tiagolobocastro@gmail.com>
@bors-openebs-mayastor
Copy link

Build failed:

We should not allow concurrent snapshot and replica operations as they might clash.
todo: allow per-resource locking
Furthermore it was observed that holding a bdev ptr whilst it's being deleted might
cause a crash, we need to determine if this was to do with it being held over an
async point, or a more general problem.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro
Copy link
Contributor Author

bors merge

@tiagolobocastro
Copy link
Contributor Author

bors cancel

@bors-openebs-mayastor
Copy link

Canceled.

Show human readable value.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro
Copy link
Contributor Author

bors merge

bors-openebs-mayastor bot pushed a commit that referenced this pull request Nov 29, 2023
1552: Various fixes for idempotency, error messages and concurrency r=tiagolobocastro a=tiagolobocastro

    fix(grpc/snapshot): snapshot grpc should use replica svc
    
    We should not allow concurrent snapshot and replica operations as they might clash.
    todo: allow per-resource locking
    Furthermore it was observed that holding a bdev ptr whilst it's being deleted might
    cause a crash, we need to determine if this was to do with it being held over an
    async point, or a more general problem.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    ci(test/wipe/replicas): allow list and wipe replicas
    
    System test is using wipe replicas, which can block list calls from the control-plane
    and thus confusing it thinking the node is not responding.
    Let's allow concurrent wipe with lists, since wiping doesn't destroy or modify
    replica metadata anyway.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    fix(io-engine/client): fix cli args
    
    Some args were not setup correctly since clap update.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    fix: bad error message
    
    ChildNotFound error had an invalid error message.
    Don't output NotFound messages for ptpl.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    fix(lvs/status): fixup missing bdev errors
    
    Allows the control-plane to determine what the inner error was.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

Co-authored-by: Tiago Castro <tiagolobocastro@gmail.com>
@bors-openebs-mayastor
Copy link

Build failed:

@tiagolobocastro
Copy link
Contributor Author

bors merge

@bors-openebs-mayastor
Copy link

Build succeeded:

@bors-openebs-mayastor bors-openebs-mayastor bot merged commit aee08d3 into develop Nov 29, 2023
4 checks passed
@bors-openebs-mayastor bors-openebs-mayastor bot deleted the fix-lvs branch November 29, 2023 20:35
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