chia7712 commented on code in PR #20214:
URL: https://github.com/apache/kafka/pull/20214#discussion_r2254538479
##########
trogdor/src/main/java/org/apache/kafka/trogdor/rest/TasksResponse.java:
##########
@@ -32,7 +31,7 @@ public class TasksResponse extends Message {
@JsonCreator
public TasksResponse(@JsonProperty("tasks") TreeMap<String, TaskState>
tasks) {
- this.tasks = Collections.unmodifiableMap((tasks == null) ? new
TreeMap<>() : tasks);
+ this.tasks = Map.copyOf((tasks == null) ? new TreeMap<>() : tasks);
Review Comment:
```java
this.tasks = tasks == null ? Map.of() : Map.copyOf(tasks);
```
##########
trogdor/src/main/java/org/apache/kafka/trogdor/task/TaskSpec.java:
##########
@@ -112,6 +111,6 @@ public String toString() {
}
protected static Map<String, String> configOrEmptyMap(Map<String, String>
config) {
- return (config == null) ? Collections.emptyMap() : config;
+ return (config == null) ? Map.of() : config;
Review Comment:
```java
return config == null ? Map.of() : config;
```
##########
trogdor/src/test/java/org/apache/kafka/trogdor/task/SampleTaskSpec.java:
##########
@@ -35,7 +34,7 @@ public SampleTaskSpec(@JsonProperty("startMs") long startMs,
@JsonProperty("error") String error) {
super(startMs, durationMs);
this.nodeToExitMs = nodeToExitMs == null ? new HashMap<>() :
Review Comment:
```java
this.nodeToExitMs = nodeToExitMs == null ? Map.of() :
Map.copyOf(nodeToExitMs);
```
##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/PartitionsSpec.java:
##########
@@ -84,7 +83,9 @@ public List<Integer> partitionNumbers() {
}
return partitionNumbers;
} else {
- return new ArrayList<>(partitionAssignments.keySet());
+ ArrayList<Integer> partitionNumbers = new
ArrayList<>(partitionAssignments.keySet());
+ partitionNumbers.sort(Integer::compareTo);
Review Comment:
Why is it being sorted?
##########
trogdor/src/main/java/org/apache/kafka/trogdor/agent/AgentClient.java:
##########
@@ -275,8 +274,7 @@ public static void main(String[] args) throws Exception {
System.out.printf("\tStart time: %s%n",
dateString(status.serverStartMs(), localOffset));
List<List<String>> lines = new ArrayList<>();
- List<String> header = new ArrayList<>(
- Arrays.asList("WORKER_ID", "TASK_ID", "STATE",
"TASK_TYPE"));
+ List<String> header = List.of("WORKER_ID", "TASK_ID",
"STATE", "TASK_TYPE");
lines.add(header);
Review Comment:
```java
lines.add(List.of("WORKER_ID", "TASK_ID", "STATE", "TASK_TYPE"));
```
##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConnectionStressSpec.java:
##########
@@ -56,7 +55,7 @@ public ConnectionStressSpec(@JsonProperty("startMs") long
startMs,
@JsonProperty("numThreads") int numThreads,
@JsonProperty("action") ConnectionStressAction action) {
super(startMs, durationMs);
- this.clientNodes = (clientNodes == null) ? Collections.emptyList() :
+ this.clientNodes = (clientNodes == null) ? List.of() :
Review Comment:
```java
this.clientNodes = clientNodes == null ? List.of() :
List.copyOf(clientNodes);
```
##########
trogdor/src/main/java/org/apache/kafka/trogdor/basic/BasicNode.java:
##########
@@ -46,7 +45,7 @@ public BasicNode(String name, String hostname, Map<String,
String> config,
public BasicNode(String name, JsonNode root) {
this.name = name;
String hostname = "localhost";
- Set<String> tags = Collections.emptySet();
+ Set<String> tags = Set.of();
Review Comment:
Perhaps it could be initialized as a `HashSet` instead?
##########
trogdor/src/main/java/org/apache/kafka/trogdor/coordinator/CoordinatorClient.java:
##########
@@ -471,8 +470,7 @@ static String prettyPrintTasksResponse(TasksResponse
response, ZoneOffset zoneOf
return "No matching tasks found.";
}
List<List<String>> lines = new ArrayList<>();
- List<String> header = new ArrayList<>(
- Arrays.asList("ID", "TYPE", "STATE", "INFO"));
+ List<String> header = List.of("ID", "TYPE", "STATE", "INFO");
Review Comment:
```java
lines.add(List.of("ID", "TYPE", "STATE", "INFO"));
```
##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/TopicsSpec.java:
##########
@@ -68,9 +67,8 @@ public void set(String name, PartitionsSpec value) {
}
public TopicsSpec immutableCopy() {
- HashMap<String, PartitionsSpec> mapCopy = new HashMap<>();
- mapCopy.putAll(map);
- return new TopicsSpec(Collections.unmodifiableMap(mapCopy));
+ HashMap<String, PartitionsSpec> mapCopy = new HashMap<>(map);
+ return new TopicsSpec(Map.copyOf(mapCopy));
Review Comment:
```java
return new TopicsSpec(Map.copyOf(map));
```
##########
trogdor/src/main/java/org/apache/kafka/trogdor/rest/TaskRequest.java:
##########
@@ -23,15 +23,12 @@
/**
* The request to /coordinator/tasks/{taskId}
*/
-public class TaskRequest {
- private final String taskId;
-
+public record TaskRequest(String taskId) {
@JsonCreator
public TaskRequest(@JsonProperty("taskId") String taskId) {
this.taskId = taskId == null ? "" : taskId;
}
- @JsonProperty
public String taskId() {
Review Comment:
This method is no longer necessary after switching to a record class, right?
##########
trogdor/src/main/java/org/apache/kafka/trogdor/rest/TasksRequest.java:
##########
@@ -69,8 +68,7 @@ public TasksRequest(@JsonProperty("taskIds")
Collection<String> taskIds,
@JsonProperty("firstEndMs") long firstEndMs,
@JsonProperty("lastEndMs") long lastEndMs,
@JsonProperty("state") Optional<TaskStateType> state) {
- this.taskIds = Collections.unmodifiableSet((taskIds == null) ?
- new HashSet<>() : new HashSet<>(taskIds));
+ this.taskIds = Set.copyOf((taskIds == null) ? new HashSet<>() : new
HashSet<>(taskIds));
Review Comment:
```java
this.taskIds = taskIds == null ? Set.of() : Set.copyOf(taskIds);
```
##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/RandomComponent.java:
##########
@@ -22,25 +22,23 @@
/**
* Contains a percent value represented as an integer between 1 and 100 and a
PayloadGenerator to specify
- * how often that PayloadGenerator should be used.
+ * how often that PayloadGenerator should be used.
*/
-public class RandomComponent {
- private final int percent;
- private final PayloadGenerator component;
-
-
+public record RandomComponent(int percent, PayloadGenerator component) {
@JsonCreator
public RandomComponent(@JsonProperty("percent") int percent,
@JsonProperty("component") PayloadGenerator
component) {
this.percent = percent;
this.component = component;
}
+ @Override
@JsonProperty
public int percent() {
Review Comment:
ditto. are those methods necessary?
##########
trogdor/src/test/java/org/apache/kafka/trogdor/common/MiniTrogdorCluster.java:
##########
@@ -161,7 +161,7 @@ public MiniTrogdorCluster build() throws Exception {
Integer.toString(node.coordinatorPort));
}
node.node = new BasicNode(entry.getKey(), node.hostname,
config,
Review Comment:
```java
node.node = new BasicNode(entry.getKey(), node.hostname, config, Set.of());
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
