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