chia7712 commented on code in PR #17965:
URL: https://github.com/apache/kafka/pull/17965#discussion_r1868249471


##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -2221,6 +2220,13 @@ AcquisitionLockTimerTask 
batchAcquisitionLockTimeoutTask() {
             return batchState.acquisitionLockTimeoutTask;
         }
 
+        private RecordState batchRollbackState() {
+            if (batchState == null) {

Review Comment:
   Apologies for my unclear comment. Let me provide an example.
   
   ### before
   ```java
           // Visible for testing.
           RecordState batchState() {
               if (batchState == null) {
                   throw new IllegalStateException("The batch state is not 
available as the offset state is maintained");
               }
               return batchState.state;
           }
   
           private RecordState batchRollbackState() {
               if (batchState == null) {
                   throw new IllegalStateException("The batch state is not 
available as the offset state is maintained");
               }
               return batchState.rollbackState();
           }
   
           // Visible for testing.
           AcquisitionLockTimerTask batchAcquisitionLockTimeoutTask() {
               if (batchState == null) {
                   throw new IllegalStateException("The batch state is not 
available as the offset state is maintained");
               }
               return batchState.acquisitionLockTimeoutTask;
           }
   
           private void archiveBatch(String newMemberId) {
               if (batchState == null) {
                   throw new IllegalStateException("The batch state is not 
available as the offset state is maintained");
               }
               batchState.archive(newMemberId);
           }
   
           private void updateAcquisitionLockTimeout(AcquisitionLockTimerTask 
acquisitionLockTimeoutTask) {
               if (batchState == null) {
                   throw new IllegalStateException("The batch state is not 
available as the offset state is maintained");
               }
               batchState.acquisitionLockTimeoutTask = 
acquisitionLockTimeoutTask;
           }
   ```
   
   ### after
   
   ```java
           // Visible for testing.
           RecordState batchState() {
               return inFlightState().state;
           }
   
           private RecordState batchRollbackState() {
               return inFlightState().rollbackState();
           }
   
           // Visible for testing.
           AcquisitionLockTimerTask batchAcquisitionLockTimeoutTask() {
               return inFlightState().acquisitionLockTimeoutTask;
           }
   
           private void archiveBatch(String newMemberId) {
               inFlightState().archive(newMemberId);
           }
   
           private void updateAcquisitionLockTimeout(AcquisitionLockTimerTask 
acquisitionLockTimeoutTask) {
               inFlightState().acquisitionLockTimeoutTask = 
acquisitionLockTimeoutTask;
           }
   ```
   
   > Even the message in each of the exceptions thrown in these functions is 
different. Hence, I am not sure how will it be useful.
   
   The methods in the above example use the same error message, so we can 
refactor them slightly.



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