hachikuji commented on a change in pull request #10914:
URL: https://github.com/apache/kafka/pull/10914#discussion_r666420857



##########
File path: 
clients/src/main/java/org/apache/kafka/common/record/DefaultRecordBatch.java
##########
@@ -167,21 +167,9 @@ public void ensureValid() {
      *         {@link RecordBatch#NO_TIMESTAMP} if the batch is empty
      */
     public long baseTimestamp() {
-        return buffer.getLong(FIRST_TIMESTAMP_OFFSET);
-    }
-
-    /**
-     * Get the timestamp of the first record in this batch. It is usually the 
create time of the record even if the
-     * timestamp type of the batch is log append time.
-     * 
-     * @return The first timestamp if a record has been appended, unless the 
delete horizon has been set
-     *         {@link RecordBatch#NO_TIMESTAMP} if the batch is empty or if 
the delete horizon is set
-     */
-    public long firstTimestamp() {
-        final long baseTimestamp = baseTimestamp();
         if (hasDeleteHorizonMs())
             return RecordBatch.NO_TIMESTAMP;

Review comment:
       Don't we want to remove this? We still need to be able to access the 
base timestamp in order to compute the successive timestamps in the record 
iterator, right?

##########
File path: 
clients/src/main/java/org/apache/kafka/common/record/DefaultRecordBatch.java
##########
@@ -167,21 +167,9 @@ public void ensureValid() {
      *         {@link RecordBatch#NO_TIMESTAMP} if the batch is empty
      */
     public long baseTimestamp() {
-        return buffer.getLong(FIRST_TIMESTAMP_OFFSET);
-    }
-
-    /**
-     * Get the timestamp of the first record in this batch. It is usually the 
create time of the record even if the
-     * timestamp type of the batch is log append time.
-     * 
-     * @return The first timestamp if a record has been appended, unless the 
delete horizon has been set
-     *         {@link RecordBatch#NO_TIMESTAMP} if the batch is empty or if 
the delete horizon is set
-     */
-    public long firstTimestamp() {
-        final long baseTimestamp = baseTimestamp();
         if (hasDeleteHorizonMs())
             return RecordBatch.NO_TIMESTAMP;
-        return baseTimestamp;
+        return buffer.getLong(FIRST_TIMESTAMP_OFFSET);

Review comment:
       Can we rename `FIRST_TIMESTAMP_OFFSET` to `BASE_TIMESTAMP_OFFSET`?




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