adixitconfluent commented on code in PR #16969:
URL: https://github.com/apache/kafka/pull/16969#discussion_r1754336687


##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -995,6 +1004,9 @@ boolean canAcquireRecords() {
      * @return A boolean which indicates whether the fetch lock is acquired.
      */
     boolean maybeAcquireFetchLock() {
+        if (partitionState() != SharePartitionState.ACTIVE) {

Review Comment:
   Hi @junrao , I am not sure of that. We acquire a write lock 
[here](https://github.com/apache/kafka/blob/trunk/core/src/main/java/kafka/server/share/SharePartition.java#L309)
 before we update the partitionState and also before reading the 
partitionState, we acquire the read lock 
[here](https://github.com/apache/kafka/blob/trunk/core/src/main/java/kafka/server/share/SharePartition.java#L1453).
 So, at all times we should be reading the latest state.



##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -995,6 +1004,9 @@ boolean canAcquireRecords() {
      * @return A boolean which indicates whether the fetch lock is acquired.
      */
     boolean maybeAcquireFetchLock() {
+        if (partitionState() != SharePartitionState.ACTIVE) {

Review Comment:
   Hi @junrao , I am not sure of that. We acquire a write lock 
[here](https://github.com/apache/kafka/blob/trunk/core/src/main/java/kafka/server/share/SharePartition.java#L309)
 before we update the partitionState and also before reading the 
partitionState, we acquire the read lock 
[here](https://github.com/apache/kafka/blob/trunk/core/src/main/java/kafka/server/share/SharePartition.java#L1453).
 So, at all times we should be reading the latest state.



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