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
librbd: sync flush should re-use existing async flush logic #18403
Conversation
f52ef52
to
7f8b7a7
Compare
} | ||
r = ctx.wait(); | ||
|
||
ictx->perfcounter->inc(l_librbd_flush); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dillaman Are you going to leave "flush" counter unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, in "qa/workunits/rbd/test_admin_socket.sh" we have a test that checks "flush" perfcounter is updated after "flush" admin socket command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah -- I thought it was already covered here [1] but it's for AIO flush. I'll combine them in the next push.
[1]
ceph/src/librbd/io/ImageRequest.cc
Line 753 in 9b18093
image_ctx.perfcounter->inc(l_librbd_aio_flush); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... consolidated
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
f2d1ac2
to
8ad2c3b
Compare
src/tools/rbd/action/Bench.cc
Outdated
int r = image.flush(); | ||
if (r < 0) { | ||
std::cerr << "rbd: failed to flush: " << cpp_strerror(r) << std::endl; | ||
return r; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want this to be fatal? E.g. previously running bench read against exclusively mapped image would succeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweaked it to be fatal for write/read-write tests and skipped on the final flush for read-only tests
8ad2c3b
to
e215a82
Compare
@dillaman |
For the new read-based bench tests, flushing prior to the start of the test will result in the exclusive lock being acquired and the object map being utilized. Signed-off-by: Jason Dillaman <dillaman@redhat.com>
e215a82
to
918df91
Compare
@trociny updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
No description provided.