satishd commented on a change in pull request #10218:
URL: https://github.com/apache/kafka/pull/10218#discussion_r596247752



##########
File path: 
clients/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentState.java
##########
@@ -87,4 +89,27 @@ public byte id() {
     public static RemoteLogSegmentState forId(byte id) {
         return STATE_TYPES.get(id);
     }
+
+    public static boolean isValidTransition(RemoteLogSegmentState srcState, 
RemoteLogSegmentState targetState) {
+        Objects.requireNonNull(targetState, "targetState can not be null");
+
+        if (srcState == null) {
+            // If the source state is null, check the target state as the 
initial state viz DELETE_PARTITION_MARKED
+            // Wanted to keep this logic simple here by taking null for 
srcState, instead of creating one more state like
+            // COPY_SEGMENT_NOT_STARTED and have the null check by caller and 
pass that state.
+            return targetState == COPY_SEGMENT_STARTED;
+        } else if (srcState == targetState) {

Review comment:
       1 -> imho, this validation method should be part of the state enum and 
it can be used by any implementation including default RLMM. 
   
   2 -> I would have preferred the suggested approach if there are many complex 
transitions but the transitions here are few and simple. 




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to