Pass the order to PreReleaseCallback #5381
Conversation
maysamyabandeh
requested review from
sagar0,
siying,
miasantreble,
riversand963,
anand1976 and
ltamasi
There was a problem hiding this comment.
@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
maysamyabandeh
force-pushed
the
prereleasecallbackindex
branch
from
1cd0457
to
794bce9
Compare
|
@maysamyabandeh has updated the pull request. Re-import the pull request |
| total_byte_size = WriteBatchInternal::AppendedByteSize( | ||
| total_byte_size, WriteBatchInternal::ByteSize(writer->batch)); | ||
| if (writer->pre_release_callback) { | ||
| pre_release_callback_cnt++; | ||
| } |
There was a problem hiding this comment.
I'm not sure which one is cheaper, ++, or this branching. I would simplify the code by having a blindly incremented variable.
There was a problem hiding this comment.
That would incorrect because if a writer doesn't have pre_release_callback, then the last pre_release_callback is labeled incorrectly.
| size_t total_byte_size = 0; | ||
| for (auto* writer : write_group) { | ||
| if (writer->CheckCallback(this)) { | ||
| total_byte_size = WriteBatchInternal::AppendedByteSize( | ||
| total_byte_size, WriteBatchInternal::ByteSize(writer->batch)); | ||
| if (writer->pre_release_callback) { | ||
| pre_release_callback_cnt++; | ||
| } |
There was a problem hiding this comment.
Same here, I feel the code will be simpler if we have a blindly incremented counter.
I have, but it has a lot of txn details and to be reviewed by someone else. I am currently polishing it. I will submit it as a separate PR soon for you to see how these are used in a real use case. |
|
Here is the PR that uses this feature: https://github.com/facebook/rocksdb/pull/5420/files#diff-2545131139890f524ea4fe013ccd7c38R811 |
|
Merged into #5420 |
The patch passes to PreReleaseCallback the order among other callbacks in the same write thread. The application could make use of it to reduce the overhead of operations that needs to be done once per write group rather than once per writer. Example is overhead of locking prepared_mutex_ in WritePrepared transactions.