WritePrepared Txn: CommitBatch #2854
Conversation
|
@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| @@ -49,6 +49,9 @@ class PessimisticTransaction : public TransactionBaseImpl { | |||
|
|
|||
| Status Commit() override; | |||
|
|
|||
| // It is basically Commit without going through Prepare phase. The write batch | |||
| // is also directly provided instead of expecting txn to gradually batch the | |||
| // transactions writes to an internal write batch. | |||
| virtual Status CommitBatch(WriteBatch* batch) = 0; | |||
There was a problem hiding this comment.
How we are using this API?
There was a problem hiding this comment.
This is a public API. Users can call it instead of going through Write + Commit steps.
There was a problem hiding this comment.
I'm wondering why this API ever existing. Fine if we don't know.
There was a problem hiding this comment.
Git shows that it was there from the very beginning. Probably someone figured it would be an optimization by avoiding the memcpy of the source write batch to the destination write batch.
There was a problem hiding this comment.
Ah I see. It make sense if it is there prior to 2PC.
| uint64_t seq_used; | ||
| auto s = db_impl_->WriteImpl(write_options_, batch, nullptr, nullptr, | ||
| no_log_ref, !disable_memtable, &seq_used); | ||
| uint64_t& prepare_seq = seq_used; |
There was a problem hiding this comment.
why reference? It is just a primitive int type.
There was a problem hiding this comment.
I needed an alias just to improve code readability and reference is the way to do. Did not mean to improve performance.
There was a problem hiding this comment.
yeah, so why not remove the reference.
There was a problem hiding this comment.
The reference shows the intention of the developer of using it just as an alias.
Implements CommitBatch and CommitWithoutPrepare for WritePreparedTxn