[GitHub] [kafka] junrao commented on a change in pull request #10424: MINOR Replaced File with Path in LogSegmentData.

[GitBox <gi...@apache.org>]

junrao commented on a change in pull request #10424:
URL: https://github.com/apache/kafka/pull/10424#discussion_r604277850



##########
File path: clients/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentMetadata.java
##########
@@ -82,26 +82,26 @@
 
     /**
      * Creates an instance with the given metadata of remote log segment.
-     *
+     * <p>
      * {@code segmentLeaderEpochs} can not be empty. If all the records in this segment belong to the same leader epoch
      * then it should have an entry with epoch mapping to start-offset of this segment.
      *
-     * @param remoteLogSegmentId  Universally unique remote log segment id.
-     * @param startOffset         Start offset of this segment (inclusive).
-     * @param endOffset           End offset of this segment (inclusive).
-     * @param maxTimestamp        Maximum timestamp in this segment.
-     * @param brokerId            Broker id from which this event is generated.
-     * @param eventTimestamp      Epoch time in milli seconds at which the remote log segment is copied to the remote tier storage.
-     * @param segmentSizeInBytes  Size of this segment in bytes.
-     * @param state               State of the respective segment of remoteLogSegmentId.
-     * @param segmentLeaderEpochs leader epochs occurred within this segment.
+     * @param remoteLogSegmentId   Universally unique remote log segment id.
+     * @param startOffset          Start offset of this segment (inclusive).
+     * @param endOffset            End offset of this segment (inclusive).
+     * @param maxTimestampMs   Maximum timestamp in milli seconds in this segment.

Review comment:
       Do you want to align the comments? Ditto below.

##########
File path: clients/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentMetadata.java
##########
@@ -271,8 +272,8 @@ public String toString() {
                ", startOffset=" + startOffset +
                ", endOffset=" + endOffset +
                ", brokerId=" + brokerId +
-               ", maxTimestamp=" + maxTimestamp +
-               ", eventTimestamp=" + eventTimestamp +
+               ", maxTimestamp=" + maxTimestampMs +

Review comment:
       Should we change the name to maxTimestampMs too? Ditto below. 

##########
File path: clients/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentMetadata.java
##########
@@ -252,15 +252,16 @@ public boolean equals(Object o) {
         }
         RemoteLogSegmentMetadata that = (RemoteLogSegmentMetadata) o;
         return startOffset == that.startOffset && endOffset == that.endOffset && brokerId == that.brokerId
-               && maxTimestamp == that.maxTimestamp && eventTimestamp == that.eventTimestamp
+               && maxTimestampMs == that.maxTimestampMs && eventTimestampMs == that.eventTimestampMs
                && segmentSizeInBytes == that.segmentSizeInBytes
                && Objects.equals(remoteLogSegmentId, that.remoteLogSegmentId)
                && Objects.equals(segmentLeaderEpochs, that.segmentLeaderEpochs) && state == that.state;
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(remoteLogSegmentId, startOffset, endOffset, brokerId, maxTimestamp, eventTimestamp,
+        return Objects.hash(remoteLogSegmentId, startOffset, endOffset, brokerId, maxTimestampMs,
+                eventTimestampMs,

Review comment:
       Could we merge this and the next line into a single line?

##########
File path: clients/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentMetadataUpdate.java
##########
@@ -99,20 +99,22 @@ public boolean equals(Object o) {
             return false;
         }
         RemoteLogSegmentMetadataUpdate that = (RemoteLogSegmentMetadataUpdate) o;
-        return eventTimestamp == that.eventTimestamp && Objects
-                .equals(remoteLogSegmentId, that.remoteLogSegmentId) && state == that.state && brokerId == that.brokerId;
+        return eventTimestampMs == that.eventTimestampMs &&
+               Objects.equals(remoteLogSegmentId, that.remoteLogSegmentId) &&
+               state == that.state &&
+               brokerId == that.brokerId;
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(remoteLogSegmentId, eventTimestamp, state, brokerId);
+        return Objects.hash(remoteLogSegmentId, eventTimestampMs, state, brokerId);
     }
 
     @Override
     public String toString() {
         return "RemoteLogSegmentMetadataUpdate{" +
                "remoteLogSegmentId=" + remoteLogSegmentId +
-               ", eventTimestamp=" + eventTimestamp +
+               ", eventTimestamp=" + eventTimestampMs +

Review comment:
       Should we change the name to eventTimestampMs too?




-- 
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:
users@infra.apache.org