BewareMyPower commented on code in PR #16940:
URL: https://github.com/apache/pulsar/pull/16940#discussion_r939773232


##########
pulsar-client-cpp/lib/MultiTopicsConsumerImpl.h:
##########
@@ -99,7 +99,7 @@ class MultiTopicsConsumerImpl : public ConsumerImplBase,
     std::map<std::string, int> topicsPartitions_;
     mutable std::mutex mutex_;
     std::mutex pendingReceiveMutex_;
-    MultiTopicsConsumerState state_ = Pending;
+    std::atomic<MultiTopicsConsumerState> state_ = {Pending};

Review Comment:
   ```suggestion
       std::atomic<MultiTopicsConsumerState> state_{Pending};
   ```
   
   `=` is not needed in brace initialization.



##########
pulsar-client-cpp/lib/PartitionedConsumerImpl.cc:
##########
@@ -389,20 +371,13 @@ void PartitionedConsumerImpl::notifyResult(CloseCallback 
closeCallback) {
     }
 }
 
-void PartitionedConsumerImpl::setState(const PartitionedConsumerState state) {
-    Lock lock(mutex_);
-    state_ = state;
-    lock.unlock();
-}
+void PartitionedConsumerImpl::setState(const PartitionedConsumerState state) { 
state_ = state; }

Review Comment:
   I think we don't need a `setState` method now with the atomic state. It's 
better to remove the `setState` method and use `state_ = xxx` instead.



-- 
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