showuon commented on code in PR #13837:
URL: https://github.com/apache/kafka/pull/13837#discussion_r1247809160


##########
storage/src/test/java/org/apache/kafka/server/log/remote/storage/LocalTieredStorageEvent.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.server.log.remote.storage;
+
+import org.apache.kafka.common.TopicPartition;
+
+import java.util.Optional;
+
+import static java.lang.String.format;
+import static java.util.Objects.requireNonNull;
+import static java.util.Optional.ofNullable;
+
+/**
+ * Represents an interaction between a broker and a second-tier storage. This 
type of event is generated
+ * by the {@link LocalTieredStorage} which is an implementation of the {@link 
RemoteStorageManager}
+ * operating in Kafka's runtime as the interface between Kafka and external 
storage systems, through
+ * which all such interactions go through.
+ */
+public final class LocalTieredStorageEvent implements 
Comparable<LocalTieredStorageEvent> {
+
+    /**
+     * The nature of the interaction.
+     */
+    public enum EventType {
+        OFFLOAD_SEGMENT,
+        FETCH_SEGMENT,
+        FETCH_OFFSET_INDEX,
+        FETCH_TIME_INDEX,
+        FETCH_TRANSACTION_INDEX,
+        FETCH_LEADER_EPOCH_CHECKPOINT,
+        FETCH_PRODUCER_SNAPSHOT,
+        DELETE_SEGMENT,
+        DELETE_PARTITION
+    }
+
+    private final int brokerId;
+    private final EventType type;
+    private final RemoteLogSegmentId segmentId;
+    private final int timestamp;
+    private final Optional<RemoteLogSegmentFileset> fileset;
+    private final Optional<RemoteLogSegmentMetadata> metadata;
+    private final int startPosition;
+    private final int endPosition;
+    private final Optional<Exception> exception;
+
+    /**
+     * Assess whether this event matches the characteristics of an event 
specified by the {@code condition}.
+     *
+     * @param condition The condition which contains the characteristics to 
match.
+     * @return true if this event matches the condition's characteristics, 
false otherwise.
+     */
+    public boolean matches(final LocalTieredStorageCondition condition) {
+        if (brokerId != condition.brokerId) {
+            return false;
+        }
+        if (condition.eventType != type) {
+            return false;
+        }
+        if 
(!segmentId.topicIdPartition().topicPartition().equals(condition.topicPartition))
 {
+            return false;
+        }
+        if (!exception.map(e -> condition.failed).orElseGet(() -> 
!condition.failed)) {
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Returns whether the provided {@code event} was created after the 
present event.
+     * This assumes a chronological ordering of events.
+     * Both events need to be generated from the same broker.
+     *
+     * @param event The event to compare
+     * @return true if the current instance was generated after the given 
{@code event},
+     *         false if events are equal or the current instance was generated 
before the
+     *         given {@code event}.
+     */
+    public boolean isAfter(final LocalTieredStorageEvent event) {
+        return event.timestamp < timestamp;
+    }
+
+    public EventType getType() {
+        return type;
+    }
+
+    public TopicPartition getTopicPartition() {
+        return segmentId.topicIdPartition().topicPartition();
+    }
+
+    @Override
+    public int compareTo(LocalTieredStorageEvent other) {
+        requireNonNull(other);
+
+        if (other.timestamp > timestamp) {
+            return -1;
+        }
+        if (other.timestamp < timestamp) {
+            return 1;
+        }
+        return 0;

Review Comment:
   We can directly return 
   ```
   return timestamp - other.timestamp;
   ```
   > Compares this object with the specified object for order. Returns a 
negative integer, zero, or a positive integer as this object is less than, 
equal to, or greater than the specified object.
   
   ref: 
https://docs.oracle.com/javase/8/docs/api/java/lang/Comparable.html#compareTo-T-
   



##########
storage/src/test/java/org/apache/kafka/server/log/remote/storage/LocalTieredStorageEvent.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.server.log.remote.storage;
+
+import org.apache.kafka.common.TopicPartition;
+
+import java.util.Optional;
+
+import static java.lang.String.format;
+import static java.util.Objects.requireNonNull;
+import static java.util.Optional.ofNullable;
+
+/**
+ * Represents an interaction between a broker and a second-tier storage. This 
type of event is generated
+ * by the {@link LocalTieredStorage} which is an implementation of the {@link 
RemoteStorageManager}
+ * operating in Kafka's runtime as the interface between Kafka and external 
storage systems, through
+ * which all such interactions go through.

Review Comment:
   Sorry, the last sentence is hard to understand: `through which all such 
interactions go through.` 
   Is there better way to describe it?



-- 
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: jira-unsubscr...@kafka.apache.org

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

Reply via email to