git-hulk commented on code in PR #1287:
URL:
https://github.com/apache/incubator-kvrocks/pull/1287#discussion_r1123924570
##########
src/storage/storage.cc:
##########
@@ -639,12 +677,41 @@ 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;
+Status Storage::BeginTxn() {
+ if (is_txn_mode_) {
+ return Status{Status::NotOK, "cannot begin a new transaction while already
in transaction mode"};
+ }
+ // The EXEC command is exclusive and shouldn't have multi transaction at the
same time,
+ // so it's fine to reset the global write batch without any lock.
+ is_txn_mode_ = true;
+ txn_write_batch_ = std::make_unique<rocksdb::WriteBatchWithIndex>();
+ return Status::OK();
+}
+Status Storage::CommitTxn() {
+ if (!is_txn_mode_) {
+ return Status{Status::NotOK, "cannot commit while not in transaction
mode"};
+ }
+ is_txn_mode_ = false;
Review Comment:
The origin Write function will check is_txn_mode status. If true, it won't
write the batch to db, so the transaction needs to be reset before committing.
But it won't have the data race since the exec command is exclusive.
There's a bit tricky in this implementation, I add a new function
`writeToDB` which won't check the transaction status and it should be more
clear. @torwig You can take a look at the commit:
https://github.com/apache/incubator-kvrocks/pull/1287/commits/b8b5a6bb4332fedd622f8f8f4248e925d122c84b
--
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]
