chia7712 commented on code in PR #20087:
URL: https://github.com/apache/kafka/pull/20087#discussion_r2217874865
##########
storage/src/test/java/org/apache/kafka/tiered/storage/specs/ConsumableSpec.java:
##########
@@ -16,33 +16,7 @@
*/
package org.apache.kafka.tiered.storage.specs;
-import java.util.Objects;
-
-public final class ConsumableSpec {
-
- private final Long fetchOffset;
- private final Integer expectedTotalCount;
- private final Integer expectedFromSecondTierCount;
-
- public ConsumableSpec(Long fetchOffset,
- Integer expectedTotalCount,
- Integer expectedFromSecondTierCount) {
- this.fetchOffset = fetchOffset;
- this.expectedTotalCount = expectedTotalCount;
- this.expectedFromSecondTierCount = expectedFromSecondTierCount;
- }
-
- public Long getFetchOffset() {
- return fetchOffset;
- }
-
- public Integer getExpectedTotalCount() {
- return expectedTotalCount;
- }
-
- public Integer getExpectedFromSecondTierCount() {
- return expectedFromSecondTierCount;
- }
+public record ConsumableSpec(Long fetchOffset, Integer expectedTotalCount,
Integer expectedFromSecondTierCount) {
Review Comment:
I checked the usage of this class, and it appears using primitive types
should be fine.
##########
storage/src/test/java/org/apache/kafka/tiered/storage/specs/FetchableSpec.java:
##########
@@ -16,26 +16,7 @@
*/
package org.apache.kafka.tiered.storage.specs;
-import java.util.Objects;
-
-public final class FetchableSpec {
-
- private final Integer sourceBrokerId;
- private final RemoteFetchCount fetchCount;
-
- public FetchableSpec(Integer sourceBrokerId,
- RemoteFetchCount fetchCount) {
- this.sourceBrokerId = sourceBrokerId;
- this.fetchCount = fetchCount;
- }
-
- public Integer getSourceBrokerId() {
- return sourceBrokerId;
- }
-
- public RemoteFetchCount getFetchCount() {
- return fetchCount;
- }
+public record FetchableSpec(Integer sourceBrokerId, RemoteFetchCount
fetchCount) {
Review Comment:
ditto
##########
storage/src/test/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentFileset.java:
##########
@@ -254,13 +254,13 @@ public static boolean deleteFilesOnly(final
Collection<File> files) {
public static boolean deleteQuietly(final File file) {
try {
- LOGGER.trace("Deleting " + file.getAbsolutePath());
+ LOGGER.trace("Deleting {}", file.getAbsolutePath());
if (!file.exists()) {
return true;
}
return file.delete();
} catch (final Exception e) {
- LOGGER.error(format("Encountered error while deleting %s",
file.getAbsolutePath()));
+ LOGGER.error("Encountered error while deleting {}",
file.getAbsolutePath());
Review Comment:
Perhaps we should add the `e` to the logger
```java
LOGGER.error("Encountered error while deleting {}", file.getAbsolutePath(),
e);
```
##########
storage/src/test/java/org/apache/kafka/tiered/storage/specs/OffloadableSpec.java:
##########
@@ -19,33 +19,9 @@
import org.apache.kafka.clients.producer.ProducerRecord;
import java.util.List;
-import java.util.Objects;
-public final class OffloadableSpec {
-
- private final Integer sourceBrokerId;
- private final Integer baseOffset;
- private final List<ProducerRecord<String, String>> records;
-
- public OffloadableSpec(Integer sourceBrokerId,
- Integer baseOffset,
- List<ProducerRecord<String, String>> records) {
- this.sourceBrokerId = sourceBrokerId;
- this.baseOffset = baseOffset;
- this.records = records;
- }
-
- public Integer getSourceBrokerId() {
- return sourceBrokerId;
- }
-
- public Integer getBaseOffset() {
- return baseOffset;
- }
-
- public List<ProducerRecord<String, String>> getRecords() {
- return records;
- }
+public record OffloadableSpec(Integer sourceBrokerId, Integer baseOffset,
Review Comment:
ditto
##########
storage/src/test/java/org/apache/kafka/tiered/storage/specs/DeletableSpec.java:
##########
@@ -18,33 +18,7 @@
import org.apache.kafka.server.log.remote.storage.LocalTieredStorageEvent;
-import java.util.Objects;
-
-public final class DeletableSpec {
-
- private final Integer sourceBrokerId;
- private final LocalTieredStorageEvent.EventType eventType;
- private final Integer eventCount;
-
- public DeletableSpec(Integer sourceBrokerId,
- LocalTieredStorageEvent.EventType eventType,
- Integer eventCount) {
- this.sourceBrokerId = sourceBrokerId;
- this.eventType = eventType;
- this.eventCount = eventCount;
- }
-
- public Integer getSourceBrokerId() {
- return sourceBrokerId;
- }
-
- public LocalTieredStorageEvent.EventType getEventType() {
- return eventType;
- }
-
- public Integer getEventCount() {
- return eventCount;
- }
+public record DeletableSpec(Integer sourceBrokerId,
LocalTieredStorageEvent.EventType eventType, Integer eventCount) {
Review Comment:
ditto
--
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
