git-hulk commented on code in PR #1287:
URL: 
https://github.com/apache/incubator-kvrocks/pull/1287#discussion_r1121027807


##########
src/storage/storage.cc:
##########
@@ -639,12 +677,33 @@ void Storage::SetIORateLimit(int64_t max_io_mb) {
 
 rocksdb::DB *Storage::GetDB() { return db_; }
 
-Status Storage::WriteToPropagateCF(const std::string &key, const std::string 
&value) {
-  rocksdb::WriteBatch batch;
+void Storage::BeginTxn() {
+  is_txn_mode_ = true;
+  txn_write_batch_ = std::make_unique<rocksdb::WriteBatchWithIndex>();
+}
+
+Status Storage::CommitTxn() {
+  is_txn_mode_ = false;
+  auto s = Write(write_opts_, txn_write_batch_->GetWriteBatch());
+  txn_write_batch_.reset();
+  if (s.ok()) {
+    return {Status::cOK};
+  }
+  return {Status::NotOK, s.ToString()};
+}
+
+std::shared_ptr<rocksdb::WriteBatchBase> Storage::GetWriteBatch() {

Review Comment:
   Yes, WriteBatchBase is a better name.
   
   >  And why we use shared_ptr rather than raw pointer here
   
   It's ok to use the raw pointer for the global transaction WriteBatch, but we 
should use the unique_ptr if it's not in transaction mode. So it needs to use 
shared_ptr if we want to put them together. BTW @PragmaTwice has offered a way 
to work around this, will have a try this week.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to