tools: New "removeall" used to remove head with snapshots #10098
Conversation
|
Still need to add test case |
dzafman
force-pushed
the
wip-16568
branch
2 times, most recently
from
5c15968
to
e60e283
Compare
It is supposed to be used as a last resort to fix a cluster that has PGs in 'incomplete' state, using the following procedure: 1) stop the osd that is primary for the incomplete PG; 2) run: ceph-objectstore-tool --data-path ... --journal-path ... --pgid $PGID --op mark-complete 3) start the osd. Fixes: ceph#10098 Signed-off-by: Mykola Golub <mgolub@mirantis.com> (cherry picked from commit 6907778) Conflicts: src/test/ceph_objectstore_tool.py (trivial) src/tools/ceph_objectstore_tool.cc (trivial)
dzafman
force-pushed
the
wip-16568
branch
2 times, most recently
from
e930805
to
dd757e7
Compare
| } | ||
|
|
||
| t.remove(coll, ghobj); | ||
| if (!dry_run) | ||
| store->apply_transaction(&osr, std::move(t)); |
There was a problem hiding this comment.
i think just checking for dry_run at this place would be enough. in other words, no need to check for dry_run every where.
There was a problem hiding this comment.
@tchaikov For consistency with the existing code I want to leave the dry_run checking as is for this pull request. However, I will consider a follow up to simplify every dry_run case throughout the application. Following your suggestion will produce better error checking.
|
@dzafman i see you attached "needs-test" label to this PR. so do you plan to add a test in qa suite or want to amend the test in |
|
@tchaikov Check out the additional changes including better testing. The cmake change isn't actually used. I used it to add "large" argument for testing in my build tree. |
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| #adds makes target/script into a test, test to check target, sets necessary environment variables | |||
| function(add_ceph_test test_name test_path) | |||
| add_test(NAME ${test_name} COMMAND ${test_path}) | |||
| add_test(NAME ${test_name} COMMAND ${test_path} ${ARGN}) | |||
There was a problem hiding this comment.
yeah, nice to have this feature! could be useful in future.
Signed-off-by: David Zafman <dzafman@redhat.com>
Signed-off-by: David Zafman <dzafman@redhat.com>
Signed-off-by: David Zafman <dzafman@redhat.com>
Signed-off-by: David Zafman <dzafman@redhat.com>
Signed-off-by: David Zafman <dzafman@redhat.com>
Signed-off-by: David Zafman <dzafman@redhat.com>
Use --force to allow remove to only remove head object Adding testing to unit test Signed-off-by: David Zafman <dzafman@redhat.com>
Signed-off-by: David Zafman <dzafman@redhat.com>
|
@tchaikov This is ready to merge once Jenkins test complete successfully. |
|
lgtm. |
Use --force to allow remove to only remove head object