mjsax commented on code in PR #14178:
URL: https://github.com/apache/kafka/pull/14178#discussion_r1289147669
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/ClientState.java:
##########
@@ -505,4 +505,15 @@ private void assertNotAssigned(final TaskId task) {
throw new IllegalArgumentException("Tried to assign task " + task
+ ", but it is already assigned: " + this);
}
}
+
+ public ClientState copy() {
Review Comment:
I think we usually use a copy-constructor for such cases.
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignor.java:
##########
@@ -36,11 +41,18 @@
public class StickyTaskAssignor implements TaskAssignor {
private static final Logger log =
LoggerFactory.getLogger(StickyTaskAssignor.class);
+ private static final int DEFAULT_STATEFUL_TRAFFIC_COST = 1;
+ private static final int DEFAULT_STATEFUL_NON_OVERLAP_COST = 10;
+ private static final int STATELESS_TRAFFIC_COST = 1;
+ private static final int STATELESS_NON_OVERLAP_COST = 0;
Review Comment:
Seems better to make the vars in the prod code accessible instead of
duplicating them?
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/RackAwareTaskAssignor.java:
##########
@@ -346,6 +346,7 @@ public long optimizeActiveTasks(final SortedSet<TaskId>
activeTasks,
log.info("Assignment before active task optimization is {}\n with cost
{}", clientStates,
activeTasksCost(activeTasks, clientStates, trafficCost,
nonOverlapCost));
+ final long startTime = System.currentTimeMillis();
Review Comment:
We should avoid calling `System.currentTimeMillis()` directly, but use the
central `Time` object instead. Not sure how complex it would be to get a handle
on it? (also below)
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/AssignmentTestUtils.java:
##########
@@ -617,21 +652,23 @@ static Map<UUID, Map<String, Optional<String>>>
getRandomProcessRacks(final int
}
Collections.shuffle(racks);
Review Comment:
Just realizing that we might want to hand in a `Random` object as second
parameter to allow us to reproduce a test run? Can you check the code for other
places where we use `shuffle` to allow us to improve it across the board?
--
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
