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
osd: FileJournal: support batch peak and pop from writeq #6701
Conversation
ec54460
to
ace620c
Compare
ace620c
to
5f11f81
Compare
@liewegas , is it make sense? It perform better in ssd. |
@jack-changtao , There are too many places where we use writeq. If do as you said, I think this would be a little complicated. @liewegas |
we need a benchmark result refer to reviewers. |
Above is the benchmark based on XinzeChi@cde544e. |
assert(write_lock.is_locked()); | ||
Mutex::Locker locker(writeq_lock); | ||
writeq.swap(items); | ||
} |
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.
batch_pop_write (it's not peek, since we're removing the items)
Hmm, was that test looking just at latency for the journal or from the client? |
the test is base on XinzeChi@cde544e. The latency is for the journal. I think XinzeChi@cde544e is good benckmark tool for filejournal? |
5f11f81
to
bf6a0ad
Compare
Ah. My concern if this buys us only a few % for just the journal and the journal is only a fraction of the overall write latency it may not be worth the risk of breaking journal replay or compatibility with other versions. Note that the new BlueStore backend won't use any of this code, so Jewel is likely the last release where FileStore will be the default. |
I think you mention #6856 instead of this patch. Tthis patch is clean and safe. |
Ah, yes, mixed them up. This is simple and clean. |
Signed-off-by: Xinze Chi <xinze@xsky.com>
But, please do keep in mind that this code isn't going to be used for too much longer. You might want to focus efforts on optimizations with the new backend(s), or in the layers above ObjectStore. |
bf6a0ad
to
a75a9a8
Compare
osd: FileJournal: support batch peak and pop from writeq Reviewed-by: Sage Weil <sage@redhat.com>
This would reduce writeq_lock contentions a lot.
Signed-off-by: Xinze Chi xinze@xsky.com