[GitHub] [kafka] badaiaqrandista closed pull request #12820: [DO NOT MERGE] 3.2 sync upstream 4 nov 2022
badaiaqrandista closed pull request #12820: [DO NOT MERGE] 3.2 sync upstream 4 nov 2022 URL: https://github.com/apache/kafka/pull/12820 -- 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
[GitHub] [kafka] badaiaqrandista opened a new pull request, #12820: [DO NOT MERGE] 3.2 sync upstream 4 nov 2022
badaiaqrandista opened a new pull request, #12820: URL: https://github.com/apache/kafka/pull/12820 NOTE: This PR should be merged using CLI to preserve commit history Based on https://confluentinc.atlassian.net/wiki/spaces/KAFKA/pages/776700227/ce-kafka+merge+process#Kafka.1 Check remotes ``` badai /Users/badai/Documents/Sources/confluentinc/kafka → git remote -v apache-kafka g...@github.com:apache/kafka.git (fetch) apache-kafka no_push (push) origin g...@github.com:confluentinc/kafka (fetch) origin g...@github.com:confluentinc/kafka (push) ``` Steps followed ``` git remote update git checkout master git pull git checkout -b 3.2-sync-upstream-4-nov-2022 origin/3.2 # Merge AK changes in from apache/kafka:3.2 → git merge apache-kafka/3.2 Auto-merging gradle.properties CONFLICT (content): Merge conflict in gradle.properties Auto-merging streams/quickstart/java/pom.xml CONFLICT (content): Merge conflict in streams/quickstart/java/pom.xml Auto-merging streams/quickstart/java/src/main/resources/archetype-resources/pom.xml CONFLICT (content): Merge conflict in streams/quickstart/java/src/main/resources/archetype-resources/pom.xml Auto-merging streams/quickstart/pom.xml CONFLICT (content): Merge conflict in streams/quickstart/pom.xml Auto-merging tests/kafkatest/__init__.py CONFLICT (content): Merge conflict in tests/kafkatest/__init__.py Auto-merging tests/kafkatest/version.py CONFLICT (content): Merge conflict in tests/kafkatest/version.py Recorded preimage for 'gradle.properties' Recorded preimage for 'streams/quickstart/java/pom.xml' Recorded preimage for 'streams/quickstart/java/src/main/resources/archetype-resources/pom.xml' Recorded preimage for 'streams/quickstart/pom.xml' Recorded preimage for 'tests/kafkatest/__init__.py' Recorded preimage for 'tests/kafkatest/version.py' Automatic merge failed; fix conflicts and then commit the result. # Fixed the conflicts and commit → git status On branch 3.2-sync-upstream-4-nov-2022 Changes to be committed: (use "git restore --staged ..." to unstage) modified: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java modified: clients/src/test/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinatorTest.java modified: docs/js/templateData.js modified: kafka-merge-pr.py modified: tests/kafkatest/version.py → git commit -m "Merged remote trancking branch into 3.2-sync-upstream-4-nov-2022" [3.2-sync-upstream-4-nov-2022 110bc01462] Merged remote trancking branch into 3.2-sync-upstream-4-nov-2022 5 files changed, 71 insertions(+), 3 deletions(-) ``` Steps remaining ``` git checkout 3.2 git pull git merge origin/3.2-sync-upstream-4-nov-2022 git push origin 3.2 ``` -- 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
[GitHub] [kafka] jeffkbkim commented on pull request #12783: KAFKA-14334: complete delayed purgatory after replication
jeffkbkim commented on PR #12783: URL: https://github.com/apache/kafka/pull/12783#issuecomment-1302757888 @dajac thanks for the review. i have addressed the comments. -- 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
[GitHub] [kafka] jeffkbkim commented on a diff in pull request #12783: KAFKA-14334: complete delayed purgatory after replication
jeffkbkim commented on code in PR #12783:
URL: https://github.com/apache/kafka/pull/12783#discussion_r1013457033
##
core/src/test/scala/unit/kafka/server/FetchRequestTest.scala:
##
@@ -51,7 +51,7 @@ class FetchRequestTest extends BaseFetchRequestTest {
def createFetchRequest(topicPartitions: Seq[TopicPartition], offsetMap:
Map[TopicPartition, Long] = Map.empty,
Review Comment:
that makes sense. i updated all the other names as well
--
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
[GitHub] [kafka] gharris1727 commented on a diff in pull request #12817: KAFKA-14346: Remove difficult to mock Plugins.compareAndSwapLoader usages
gharris1727 commented on code in PR #12817:
URL: https://github.com/apache/kafka/pull/12817#discussion_r1013212435
##
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java:
##
@@ -155,10 +143,22 @@ public LoaderSwap withClassLoader(ClassLoader loader) {
}
}
+public Runnable withClassLoader(ClassLoader classLoader, Runnable
operation) {
Review Comment:
I had a version which did that but felt that it unnecessarily duplicated the
other withClassLoader :)
##
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java:
##
@@ -421,19 +421,24 @@ private void removeConnectorTasks(String connName) {
}
private void updateConnectorTasks(String connName) {
-if (!worker.isRunning(connName)) {
-log.info("Skipping update of connector {} since it is not
running", connName);
-return;
-}
+try {
+if (!worker.isRunning(connName)) {
+log.info("Skipping update of connector {} since it is not
running", connName);
+return;
+}
-List> newTaskConfigs =
recomputeTaskConfigs(connName);
-List> oldTaskConfigs =
configState.allTaskConfigs(connName);
+List> newTaskConfigs =
recomputeTaskConfigs(connName);
+List> oldTaskConfigs =
configState.allTaskConfigs(connName);
-if (!newTaskConfigs.equals(oldTaskConfigs)) {
-removeConnectorTasks(connName);
-List> rawTaskConfigs =
reverseTransform(connName, configState, newTaskConfigs);
-configBackingStore.putTaskConfigs(connName, rawTaskConfigs);
-createConnectorTasks(connName);
+if (!newTaskConfigs.equals(oldTaskConfigs)) {
+removeConnectorTasks(connName);
+List> rawTaskConfigs =
reverseTransform(connName, configState, newTaskConfigs);
+configBackingStore.putTaskConfigs(connName, rawTaskConfigs);
+createConnectorTasks(connName);
+}
+} catch (Throwable t) {
+// TODO: when this throws errors where do they go
+log.error("Unable to update connector tasks", t);
Review Comment:
yeah something i need to follow up on but doesn't belong in this PR.
I had some bad mocks which caused a 1000s wait while this method swallowed
the errors.
##
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConnector.java:
##
@@ -116,14 +115,12 @@ public void run() {
LoggingContext.clear();
try (LoggingContext loggingContext =
LoggingContext.forConnector(connName)) {
-ClassLoader savedLoader = Plugins.compareAndSwapLoaders(loader);
Review Comment:
I really wanted to make this happen, but it turns out the getter is used by
the Worker to switch into the proper classloader for the connector before
interacting with the WorkerConnector.
There was an alternative to pull the plugin loader from the Connector object
itself, but this same alternative did not exist for the WorkerTask, as the
`task` field is only a member of the subclasses, and not of the WorkerTask
itself.
--
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
[GitHub] [kafka] qingwei91 commented on pull request #12166: KAFKA-13817 Always sync nextTimeToEmit with wall clock
qingwei91 commented on PR #12166: URL: https://github.com/apache/kafka/pull/12166#issuecomment-1302692988 @mjsax sorry, I will try to pick this back up this weekend -- 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
[GitHub] [kafka] jeffkbkim commented on a diff in pull request #12783: KAFKA-14334: complete delayed purgatory after replication
jeffkbkim commented on code in PR #12783:
URL: https://github.com/apache/kafka/pull/12783#discussion_r1013413433
##
core/src/main/scala/kafka/server/ReplicaFetcherThread.scala:
##
@@ -132,9 +139,22 @@ class ReplicaFetcherThread(name: String,
brokerTopicStats.updateReplicationBytesIn(records.sizeInBytes)
+val highWatermarkChanged =
log.maybeUpdateHighWatermark(partitionData.highWatermark)
+if (highWatermarkChanged) {
+ logAppendInfo.foreach { _ => partitionsWithNewHighWatermark +=
topicPartition }
+ if (logTrace)
+trace(s"Follower updated replica high watermark for partition
$topicPartition to ${partitionData.highWatermark}")
Review Comment:
much more elegant. thanks for the suggestion!
--
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
[GitHub] [kafka] ashmeet13 commented on a diff in pull request #12684: KAFKA-14254; Format timestamps as dates in logs
ashmeet13 commented on code in PR #12684:
URL: https://github.com/apache/kafka/pull/12684#discussion_r1013311309
##
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##
@@ -1450,4 +1453,15 @@ public static String[] enumOptions(Class> enumClass) {
.toArray(String[]::new);
}
+/**
+ * Convert time instant to readable string for logging
+ * @param timestamp the timestamp of the instant to be converted.
+ *
+ * @return string value of a given timestamp in the format "-MM-dd
HH:mm:ss,SSS"
+ */
+public static String toLogDateTimeFormat(long timestamp) {
+final DateTimeFormatter dateTimeFormatter =
DateTimeFormatter.ofPattern("-MM-dd HH:mm:ss,SSS");
Review Comment:
Hi @cadonna, Thank you for the review!
I have added the change for this. A few sample logs with the updated
datetime format -
```
[2022-11-04 00:36:28,517] INFO stream-thread
[stream-thread-test-87bf53a8-54f2-485f-a4b6-acdbec0a8b3d-StreamThread-1]
Triggering the followup rebalance scheduled for 2022-11-04 00:36:28,511 +05:30.
(org.apache.kafka.streams.processor.internals.StreamThread:614)
```
```
[2022-11-04 00:46:52,220] INFO stream-thread [] Requesting followup
rebalance be scheduled by consumer10 for 2022-11-04 00:56:52,212 +05:30 to
probe for caught-up replica tasks.
(org.apache.kafka.streams.processor.internals.StreamsPartitionAssignor:960)
```
--
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
[GitHub] [kafka] ashmeet13 commented on a diff in pull request #12684: KAFKA-14254; Format timestamps as dates in logs
ashmeet13 commented on code in PR #12684:
URL: https://github.com/apache/kafka/pull/12684#discussion_r1013311309
##
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##
@@ -1450,4 +1453,15 @@ public static String[] enumOptions(Class> enumClass) {
.toArray(String[]::new);
}
+/**
+ * Convert time instant to readable string for logging
+ * @param timestamp the timestamp of the instant to be converted.
+ *
+ * @return string value of a given timestamp in the format "-MM-dd
HH:mm:ss,SSS"
+ */
+public static String toLogDateTimeFormat(long timestamp) {
+final DateTimeFormatter dateTimeFormatter =
DateTimeFormatter.ofPattern("-MM-dd HH:mm:ss,SSS");
Review Comment:
Hi @cadonna, I have added the change for this. A few sample logs with the
updated datetime format -
```
[2022-11-04 00:36:28,517] INFO stream-thread
[stream-thread-test-87bf53a8-54f2-485f-a4b6-acdbec0a8b3d-StreamThread-1]
Triggering the followup rebalance scheduled for 2022-11-04 00:36:28,511 +05:30.
(org.apache.kafka.streams.processor.internals.StreamThread:614)
```
```
[2022-11-04 00:46:52,220] INFO stream-thread [] Requesting followup
rebalance be scheduled by consumer10 for 2022-11-04 00:56:52,212 +05:30 to
probe for caught-up replica tasks.
(org.apache.kafka.streams.processor.internals.StreamsPartitionAssignor:960)
```
--
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
[jira] [Updated] (KAFKA-14354) Add delete callback method to Connector API
[ https://issues.apache.org/jira/browse/KAFKA-14354?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Hector Geraldino updated KAFKA-14354: - Description: It would be useful to have a callback method added to the Connector API, so connectors extending the SourceConnector and SinkConnector classes can be notified when their connector instance is being deleted. This will give a chance to connectors to perform any cleanup tasks (e.g. deleting external resources, or deleting offsets) before the connector is completely removed from the cluster. (was: KIP-795: https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for+AbstractCoordinator The AbstractCoordinator should have a companion public interface that is part of Kafka's public API, so backwards compatibility can be maintained in future versions of the client libraries) > Add delete callback method to Connector API > --- > > Key: KAFKA-14354 > URL: https://issues.apache.org/jira/browse/KAFKA-14354 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Hector Geraldino >Assignee: Hector Geraldino >Priority: Minor > > It would be useful to have a callback method added to the Connector API, so > connectors extending the SourceConnector and SinkConnector classes can be > notified when their connector instance is being deleted. This will give a > chance to connectors to perform any cleanup tasks (e.g. deleting external > resources, or deleting offsets) before the connector is completely removed > from the cluster. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Updated] (KAFKA-14354) Add delete callback method to Connector API
[ https://issues.apache.org/jira/browse/KAFKA-14354?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Hector Geraldino updated KAFKA-14354: - Priority: Minor (was: Major) > Add delete callback method to Connector API > --- > > Key: KAFKA-14354 > URL: https://issues.apache.org/jira/browse/KAFKA-14354 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Hector Geraldino >Assignee: Hector Geraldino >Priority: Minor > > KIP-795: > https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for+AbstractCoordinator > The AbstractCoordinator should have a companion public interface that is part > of Kafka's public API, so backwards compatibility can be maintained in future > versions of the client libraries -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Updated] (KAFKA-14354) Add delete callback method to Connector API
[ https://issues.apache.org/jira/browse/KAFKA-14354?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Hector Geraldino updated KAFKA-14354: - Component/s: KafkaConnect (was: clients) > Add delete callback method to Connector API > --- > > Key: KAFKA-14354 > URL: https://issues.apache.org/jira/browse/KAFKA-14354 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Hector Geraldino >Assignee: Hector Geraldino >Priority: Major > > KIP-795: > https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for+AbstractCoordinator > The AbstractCoordinator should have a companion public interface that is part > of Kafka's public API, so backwards compatibility can be maintained in future > versions of the client libraries -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (KAFKA-14354) Add delete callback method to Connector API
Hector Geraldino created KAFKA-14354: Summary: Add delete callback method to Connector API Key: KAFKA-14354 URL: https://issues.apache.org/jira/browse/KAFKA-14354 Project: Kafka Issue Type: Improvement Components: clients Reporter: Hector Geraldino Assignee: Hector Geraldino KIP-795: https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for+AbstractCoordinator The AbstractCoordinator should have a companion public interface that is part of Kafka's public API, so backwards compatibility can be maintained in future versions of the client libraries -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Resolved] (KAFKA-13434) Add a public API for AbstractCoordinator
[ https://issues.apache.org/jira/browse/KAFKA-13434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Hector Geraldino resolved KAFKA-13434. -- Resolution: Won't Do KIP has been discarded > Add a public API for AbstractCoordinator > > > Key: KAFKA-13434 > URL: https://issues.apache.org/jira/browse/KAFKA-13434 > Project: Kafka > Issue Type: Improvement > Components: clients >Reporter: Hector Geraldino >Assignee: Hector Geraldino >Priority: Major > > KIP-795: > https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for+AbstractCoordinator > The AbstractCoordinator should have a companion public interface that is part > of Kafka's public API, so backwards compatibility can be maintained in future > versions of the client libraries -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [kafka] hachikuji commented on pull request #12819: MINOR: Change system test console consumer default log level
hachikuji commented on PR #12819: URL: https://github.com/apache/kafka/pull/12819#issuecomment-1302513330 @jsancio Maybe it's good enough to post one test which uses the console consumer service? -- 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
[GitHub] [kafka] jsancio commented on pull request #12819: MINOR: Change system test console consumer default log level
jsancio commented on PR #12819: URL: https://github.com/apache/kafka/pull/12819#issuecomment-1302501942 @hachikuji Are you planning to run Confluent' branch builder job and share the results? -- 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
[GitHub] [kafka] hachikuji opened a new pull request, #12819: MINOR: Change system test console consumer default log level
hachikuji opened a new pull request, #12819: URL: https://github.com/apache/kafka/pull/12819 For tests which use the console consumer service, we are currently enabling TRACE logging by default. I have seen some system tests where this produces GBs of logging. A better default is probably DEBUG. ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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
[GitHub] [kafka] vamossagar12 commented on pull request #12756: Kafka 12960: Follow up Commit to filter expired records from Windowed/Session Stores
vamossagar12 commented on PR #12756: URL: https://github.com/apache/kafka/pull/12756#issuecomment-1302449679 @vpapavas , @ableegoldman I was able to fix the failing test cases. Plz review whenever you get the chance. I ran the tests locally and they seemed to have passed. -- 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
[GitHub] [kafka] C0urante commented on a diff in pull request #12544: KAFKA-14098: Add meaningful default client IDs for Connect workers
C0urante commented on code in PR #12544:
URL: https://github.com/apache/kafka/pull/12544#discussion_r1013182540
##
connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java:
##
@@ -106,6 +108,7 @@ public Connect startConnect(Map
workerProps) {
// Create the admin client to be shared by all backing stores.
Map adminProps = new HashMap<>(config.originals());
ConnectUtils.addMetricsContextProperties(adminProps, config,
kafkaClusterId);
+adminProps.putIfAbsent(CLIENT_ID_CONFIG, "connect-cluster-" +
config.groupId());
Review Comment:
Hmm... yeah, that's fair. I'd still like to do _something_ if the user
doesn't provide a `client.id`, even if it's no longer unique in a Connect
cluster; could we just leave out that portion and do something like
`-configs`? Tweaked the PR to use that logic; LMKWYT
--
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
[GitHub] [kafka] C0urante commented on a diff in pull request #12544: KAFKA-14098: Add meaningful default client IDs for Connect workers
C0urante commented on code in PR #12544:
URL: https://github.com/apache/kafka/pull/12544#discussion_r1013182540
##
connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java:
##
@@ -106,6 +108,7 @@ public Connect startConnect(Map
workerProps) {
// Create the admin client to be shared by all backing stores.
Map adminProps = new HashMap<>(config.originals());
ConnectUtils.addMetricsContextProperties(adminProps, config,
kafkaClusterId);
+adminProps.putIfAbsent(CLIENT_ID_CONFIG, "connect-cluster-" +
config.groupId());
Review Comment:
Hmm... yeah, that's fair. I'd still like to do _something_ if the user
doesn't provide a `client.id`; could we just leave out that portion and do
something like `-configs`? Tweaked the PR to use that logic; LMKWYT
--
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
[GitHub] [kafka] cmccabe commented on a diff in pull request #12776: KAFKA-14327: Unify KRaft snapshot generation between broker and controller
cmccabe commented on code in PR #12776:
URL: https://github.com/apache/kafka/pull/12776#discussion_r1013166571
##
server-common/src/test/java/org/apache/kafka/queue/KafkaEventQueueTest.java:
##
@@ -240,4 +242,29 @@ public void handleException(Throwable e) {
assertEquals(RejectedExecutionException.class, assertThrows(
ExecutionException.class, () ->
future.get()).getCause().getClass());
}
-}
+
+@Test
+public void testEmpty() throws Exception {
+KafkaEventQueue queue = new KafkaEventQueue(Time.SYSTEM, new
LogContext(),
+"testEmpty");
+assertTrue(queue.isEmpty());
+CompletableFuture future = new CompletableFuture<>();
+queue.append(() -> future.get());
+assertFalse(queue.isEmpty());
Review Comment:
This had a bug, which is that it wasn't taking into account the currently
running event in its calculation of "empty". I fixed this bug and the test
should be working now. I don't think we need the waits (except in the one
position)
--
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
[GitHub] [kafka] C0urante merged pull request #12409: KAFKA-14058: Migrate ExactlyOnceWorkerSourceTaskTest from EasyMock and PowerMock to Mockito
C0urante merged PR #12409: URL: https://github.com/apache/kafka/pull/12409 -- 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
[GitHub] [kafka] C0urante commented on pull request #12409: KAFKA-14058: Migrate ExactlyOnceWorkerSourceTaskTest from EasyMock and PowerMock to Mockito
C0urante commented on PR #12409: URL: https://github.com/apache/kafka/pull/12409#issuecomment-1302382288 Thanks @mimaison. Test failures appear unrelated, going to merge. -- 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
[jira] [Resolved] (KAFKA-14058) Replace EasyMock and PowerMock with Mockito in ExactlyOnceWorkerSourceTaskTest
[ https://issues.apache.org/jira/browse/KAFKA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Chris Egerton resolved KAFKA-14058. --- Fix Version/s: 3.4.0 Resolution: Fixed > Replace EasyMock and PowerMock with Mockito in ExactlyOnceWorkerSourceTaskTest > -- > > Key: KAFKA-14058 > URL: https://issues.apache.org/jira/browse/KAFKA-14058 > Project: Kafka > Issue Type: Sub-task > Components: KafkaConnect >Reporter: Chris Egerton >Assignee: Chris Egerton >Priority: Minor > Fix For: 3.4.0 > > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Updated] (KAFKA-14132) Remaining PowerMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14132:
--
Description:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven])
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven])
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven])
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# {color:#FF8B00}KafkaBasedLogTest{color} (owner: [~mdedetrich-aiven])
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# {color:#FF8B00}KafkaBasedLogTest{color} (owner: [~mdedetrich-aiven] )
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
> Remaining PowerMock to Mockito tests
>
>
> Key: KAFKA-14132
> URL: https://issues.apache.org/jira/browse/KAFKA-14132
> Project: Kafka
> Issue Type: Sub-task
>Reporter: Christo Lolov
>Assignee: Christo Lolov
>Priority: Major
>
> {color:#de350b}Some of the tests below use EasyMock as well. For those
> migrate both PowerMock and EasyMock to Mockito.{color}
> Unless stated in brackets the tests are in the connect module.
> A list of tests which still require to be moved from PowerMock to Mockito as
> of 2nd of August 2022 which do not have a Jira issue and do not have pull
> requests I am aware of which are opened:
> {color:#ff8b00}InReview{color}
> {color:#00875a}Merged{color}
> # {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
> # SourceTaskOffsetCommiterTest
> # {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
> # WorkerSinkTaskTest (owner: Divij) *WIP*
> # WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
> # {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
> # {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
> # {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
> # {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~
[jira] [Updated] (KAFKA-14132) Remaining PowerMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14132:
--
Description:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# {color:#FF8B00}KafkaBasedLogTest{color} (owner: [~mdedetrich-aiven] )
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# KafkaBasedLogTest (owner: [~mdedetrich-aiven] )
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
> Remaining PowerMock to Mockito tests
>
>
> Key: KAFKA-14132
> URL: https://issues.apache.org/jira/browse/KAFKA-14132
> Project: Kafka
> Issue Type: Sub-task
>Reporter: Christo Lolov
>Assignee: Christo Lolov
>Priority: Major
>
> {color:#de350b}Some of the tests below use EasyMock as well. For those
> migrate both PowerMock and EasyMock to Mockito.{color}
> Unless stated in brackets the tests are in the connect module.
> A list of tests which still require to be moved from PowerMock to Mockito as
> of 2nd of August 2022 which do not have a Jira issue and do not have pull
> requests I am aware of which are opened:
> {color:#ff8b00}InReview{color}
> {color:#00875a}Merged{color}
> # {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
> # SourceTaskOffsetCommiterTest
> # {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
> # WorkerSinkTaskTest (owner: Divij) *WIP*
> # WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
> # {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
> # {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
> # {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
> # {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
> #
[GitHub] [kafka] lucasbru commented on a diff in pull request #12795: KAFKA-14299: Initialize directly after handleAssignment
lucasbru commented on code in PR #12795:
URL: https://github.com/apache/kafka/pull/12795#discussion_r1013142335
##
streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java:
##
@@ -395,8 +395,10 @@ private void createNewTasks(final Map> activeTasksTo
tasks.addActiveTasks(newActiveTasks);
tasks.addStandbyTasks(newStandbyTask);
} else {
-tasks.addPendingTaskToInit(newActiveTasks);
-tasks.addPendingTaskToInit(newStandbyTask);
+final Map taskInitExceptions = new
LinkedHashMap<>();
+Stream.concat(newActiveTasks.stream(), newStandbyTask.stream())
+.forEach(t -> addTaskToStateUpdater(t,
taskInitExceptions));
Review Comment:
If we initialize inside the state updater (lazily using
`initializeIfNeeded`, before restoration), we should be able to avoid any extra
handling for these tasks. In `handleAssignment`, we can directly add the
uninitialized tasks to the state updater.
The advantages would be
- No `pendingTaskToInit` required, so some simplification.
- We don't crash when two rebalances happen in one round of polling (why I
opened this PR, see description)
- After a rebalance, all tasks that remained with this instance can
immediately continue processing, and we do not have to wait for new tasks to
initialize their rocksdb (this is more the cherry on top)
The last option that I see to fix the `IllegalStateException` (besides
moving initialization to the poll-phase or to the state updater) is that we can
take care to not recreate tasks that are already in `pendingTaskToInit`, but it
could get complicated. It can happen that the input partitions change, in which
case we need to move the already created task but not initialized task to
`pendingTaskToUpdateInputPartitions` - but at the same time remember that the
task isn't initialized yet ... not sure exactly how to handle these things
correctly, but it looks like it will just add more corner cases to think about.
--
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
[jira] [Updated] (KAFKA-14132) Remaining PowerMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14132:
--
Description:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# KafkaBasedLogTest (owner: [~mdedetrich-aiven] )
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
[https://github.com/apache/kafka/pull/12735]
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# KafkaBasedLogTest (owner: [~mdedetrich-aiven] )
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
> Remaining PowerMock to Mockito tests
>
>
> Key: KAFKA-14132
> URL: https://issues.apache.org/jira/browse/KAFKA-14132
> Project: Kafka
> Issue Type: Sub-task
>Reporter: Christo Lolov
>Assignee: Christo Lolov
>Priority: Major
>
> {color:#de350b}Some of the tests below use EasyMock as well. For those
> migrate both PowerMock and EasyMock to Mockito.{color}
> Unless stated in brackets the tests are in the connect module.
> A list of tests which still require to be moved from PowerMock to Mockito as
> of 2nd of August 2022 which do not have a Jira issue and do not have pull
> requests I am aware of which are opened:
> {color:#ff8b00}InReview{color}
> {color:#00875a}Merged{color}
> # {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
> # SourceTaskOffsetCommiterTest
> # {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
> # WorkerSinkTaskTest (owner: Divij) *WIP*
> # WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
> # {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
> # {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
> # {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
> # {color:#00875a}WorkerErrantRecordReporterTest{color} (ow
[jira] [Updated] (KAFKA-14132) Remaining PowerMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14132:
--
Description:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
[https://github.com/apache/kafka/pull/12735]
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# KafkaBasedLogTest (owner: [~mdedetrich-aiven] )
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}Some of the tests below use EasyMock as well. For those migrate
both PowerMock and EasyMock to Mockito.{color}
Unless stated in brackets the tests are in the connect module.
A list of tests which still require to be moved from PowerMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}InReview{color}
{color:#00875a}Merged{color}
# {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
[https://github.com/apache/kafka/pull/12735]
# SourceTaskOffsetCommiterTest
# {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
# WorkerSinkTaskTest (owner: Divij) *WIP*
# WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
# {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
# {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya])
# {color:#00875a}RetryWithToleranceOperatorTest{color} (owner: [~yash.mayya])
# {color:#00875a}WorkerErrantRecordReporterTest{color} (owner: [~yash.mayya])
# {color:#ff8b00}ConnectorsResourceTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12725])
# {color:#ff8b00}StandaloneHerderTest{color} (owner: [~mdedetrich-aiven] )
([https://github.com/apache/kafka/pull/12728])
# KafkaConfigBackingStoreTest (owner: [~mdedetrich-aiven] )
# {color:#ff8b00}KafkaOffsetBackingStoreTest{color} (owner: Christo)
([https://github.com/apache/kafka/pull/12418])
# KafkaBasedLogTest (owner: [~mdedetrich-aiven] )
# RetryUtilTest (owner: [~mdedetrich-aiven] )
# RepartitionTopicTest (streams) (owner: Christo)
# StateManagerUtilTest (streams) (owner: Christo)
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
> Remaining PowerMock to Mockito tests
>
>
> Key: KAFKA-14132
> URL: https://issues.apache.org/jira/browse/KAFKA-14132
> Project: Kafka
> Issue Type: Sub-task
>Reporter: Christo Lolov
>Assignee: Christo Lolov
>Priority: Major
>
> {color:#de350b}Some of the tests below use EasyMock as well. For those
> migrate both PowerMock and EasyMock to Mockito.{color}
> Unless stated in brackets the tests are in the connect module.
> A list of tests which still require to be moved from PowerMock to Mockito as
> of 2nd of August 2022 which do not have a Jira issue and do not have pull
> requests I am aware of which are opened:
> {color:#ff8b00}InReview{color}
> {color:#00875a}Merged{color}
> # {color:#ff8b00}ErrorHandlingTaskTest{color} (owner: [~shekharrajak])
> [https://github.com/apache/kafka/pull/12735]
> # SourceTaskOffsetCommiterTest
> # {color:#00875a}WorkerMetricsGroupTest{color} (owner: Divij)
> # WorkerSinkTaskTest (owner: Divij) *WIP*
> # WorkerSinkTaskThreadedTest (owner: Divij) *WIP*
> # {color:#00875a}WorkerTaskTest{color} (owner: [~yash.mayya])
> # {color:#00875a}ErrorReporterTest{color} (owner: [~yash.mayya]
[jira] [Updated] (KAFKA-14133) Remaining EasyMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14133?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14133:
--
Description:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (connect) (owner:
[~yash.mayya] )
# {color:#00875a}KStreamFlatTransformTest{color} (owner: Christo)
# {color:#00875a}KStreamFlatTransformValuesTest{color} (owner: Christo)
# {color:#00875a}KStreamPrintTest{color} (owner: Christo)
# {color:#00875a}KStreamRepartitionTest{color} (owner: Christo)
# {color:#00875a}MaterializedInternalTest{color} (owner: Christo)
# {color:#00875a}TransformerSupplierAdapterTest{color} (owner: Christo)
# {color:#00875a}KTableSuppressProcessorMetricsTest{color} (owner: Christo)
# {color:#00875a}ClientUtilsTest{color} (owner: Christo)
# {color:#00875a}HighAvailabilityStreamsPartitionAssignorTest{color} (owner:
Christo)
# {color:#00875A}TopologyTest{color} (owner: Christo)
# {color:#00875A}KTableSuppressProcessorTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingSessionBytesStoreTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingTimestampedWindowBytesStoreTest{color} (owner:
Christo)
# {color:#00875A}ChangeLoggingWindowBytesStoreTest{color} (owner: Christo)
# {color:#00875A}MeteredTimestampedWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamsRebalanceListenerTest{color} (owner: Christo)
# {color:#ff8b00}TimestampedKeyValueStoreMaterializerTest{color} (owner:
Christo)
# {color:#ff8b00}CachingInMemoryKeyValueStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingInMemorySessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentSessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingKeyValueBytesStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingTimestampedKeyValueBytesStoreTest{color} (owner:
Christo)
# {color:#ff8b00}CompositeReadOnlyWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}KeyValueStoreBuilderTest{color} (owner: Christo)
# {color:#ff8b00}RocksDBStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamThreadStateStoreProviderTest{color} (owner: Christo)
# {color:#ff8b00}TaskManagerTest{color} (owner: Christo)
# {color:#FF8B00}InternalTopicManagerTest{color} (owner: Christo)
# {color:#FF8B00}ProcessorContextImplTest{color} (owner: Christo)
# {color:#FF8B00}WriteConsistencyVectorTest{color} (owner: Christo)
# {color:#FF8B00}StreamsAssignmentScaleTest{color} (owner: Christo)
# {color:#FF8B00}StreamsPartitionAssignorTest{color} (owner: Christo)
# {color:#FF8B00}AssignmentTestUtils{color} (owner: Christo)
# {color:#FF8B00}ProcessorStateManagerTest{color} (owner: Matthew)
# {color:#FF8B00}StandbyTaskTest{color} (owner: Matthew)
# {color:#FF8B00}StoreChangelogReaderTest{color} (owner: Matthew)
# {color:#FF8B00}StreamTaskTest{color} (owner: Matthew)
# {color:#FF8B00}StreamThreadTest{color} (owner: Matthew)
# {color:#FF8B00}StreamsMetricsImplTest{color} (owner: Dalibor) (Captured in
https://issues.apache.org/jira/browse/KAFKA-12947)
# {color:#FF8B00}TimeOrderedCachingPersistentWindowStoreTest{color} (owner:
[~shekharrajak])
# {color:#FF8B00}TimeOrderedWindowStoreTest{color} (owners: [~shekharrajak])
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{col
[jira] [Updated] (KAFKA-14133) Remaining EasyMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14133?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14133:
--
Description:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (connect) (owner:
[~yash.mayya] )
# {color:#00875a}KStreamFlatTransformTest{color} (owner: Christo)
# {color:#00875a}KStreamFlatTransformValuesTest{color} (owner: Christo)
# {color:#00875a}KStreamPrintTest{color} (owner: Christo)
# {color:#00875a}KStreamRepartitionTest{color} (owner: Christo)
# {color:#00875a}MaterializedInternalTest{color} (owner: Christo)
# {color:#00875a}TransformerSupplierAdapterTest{color} (owner: Christo)
# {color:#00875a}KTableSuppressProcessorMetricsTest{color} (owner: Christo)
# {color:#00875a}ClientUtilsTest{color} (owner: Christo)
# {color:#00875a}HighAvailabilityStreamsPartitionAssignorTest{color} (owner:
Christo)
# {color:#00875A}TopologyTest{color} (owner: Christo)
# {color:#00875A}KTableSuppressProcessorTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingSessionBytesStoreTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingTimestampedWindowBytesStoreTest{color} (owner:
Christo)
# {color:#00875A}ChangeLoggingWindowBytesStoreTest{color} (owner: Christo)
# {color:#00875A}MeteredTimestampedWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamsRebalanceListenerTest{color} (owner: Christo)
# {color:#ff8b00}TimestampedKeyValueStoreMaterializerTest{color} (owner:
Christo)
# {color:#ff8b00}CachingInMemoryKeyValueStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingInMemorySessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentSessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingKeyValueBytesStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingTimestampedKeyValueBytesStoreTest{color} (owner:
Christo)
# {color:#ff8b00}CompositeReadOnlyWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}KeyValueStoreBuilderTest{color} (owner: Christo)
# {color:#ff8b00}RocksDBStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamThreadStateStoreProviderTest{color} (owner: Christo)
# {color:#ff8b00}TaskManagerTest{color} (owner: Christo)
# {color:#FF8B00}InternalTopicManagerTest{color} (owner: Christo)
# {color:#FF8B00}ProcessorContextImplTest{color} (owner: Christo)
# {color:#FF8B00}WriteConsistencyVectorTest{color} (owner: Christo)
# {color:#FF8B00}StreamsAssignmentScaleTest{color} (owner: Christo)
# {color:#FF8B00}StreamsPartitionAssignorTest{color} (owner: Christo)
# {color:#FF8B00}AssignmentTestUtils{color} (owner: Christo)
# {color:#FF8B00}ProcessorStateManagerTest{color} (owner: Matthew)
# {color:#FF8B00}StandbyTaskTest{color} (owner: Matthew)
# {color:#FF8B00}StoreChangelogReaderTest{color} (owner: Matthew)
# {color:#FF8B00}StreamTaskTest{color} (owner: Matthew)
# {color:#FF8B00}StreamThreadTest{color} (owner: Matthew)
# -StreamsMetricsImplTest (owner:- [~shekharrajak]{-}){-} (Captured in
https://issues.apache.org/jira/browse/KAFKA-12947)
# {color:#ff8b00}TimeOrderedCachingPersistentWindowStoreTest{color} (owner:
[~shekharrajak])
# {color:#FF8B00}TimeOrderedWindowStoreTest{color} (owners: [~shekharrajak])
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (c
[jira] [Updated] (KAFKA-14133) Remaining EasyMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14133?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14133:
--
Description:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (connect) (owner:
[~yash.mayya] )
# {color:#00875a}KStreamFlatTransformTest{color} (owner: Christo)
# {color:#00875a}KStreamFlatTransformValuesTest{color} (owner: Christo)
# {color:#00875a}KStreamPrintTest{color} (owner: Christo)
# {color:#00875a}KStreamRepartitionTest{color} (owner: Christo)
# {color:#00875a}MaterializedInternalTest{color} (owner: Christo)
# {color:#00875a}TransformerSupplierAdapterTest{color} (owner: Christo)
# {color:#00875a}KTableSuppressProcessorMetricsTest{color} (owner: Christo)
# {color:#00875a}ClientUtilsTest{color} (owner: Christo)
# {color:#00875a}HighAvailabilityStreamsPartitionAssignorTest{color} (owner:
Christo)
# {color:#00875A}TopologyTest{color} (owner: Christo)
# {color:#00875A}KTableSuppressProcessorTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingSessionBytesStoreTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingTimestampedWindowBytesStoreTest{color} (owner:
Christo)
# {color:#00875A}ChangeLoggingWindowBytesStoreTest{color} (owner: Christo)
# {color:#00875A}MeteredTimestampedWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamsRebalanceListenerTest{color} (owner: Christo)
# {color:#ff8b00}TimestampedKeyValueStoreMaterializerTest{color} (owner:
Christo)
# {color:#ff8b00}CachingInMemoryKeyValueStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingInMemorySessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentSessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingKeyValueBytesStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingTimestampedKeyValueBytesStoreTest{color} (owner:
Christo)
# {color:#ff8b00}CompositeReadOnlyWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}KeyValueStoreBuilderTest{color} (owner: Christo)
# {color:#ff8b00}RocksDBStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamThreadStateStoreProviderTest{color} (owner: Christo)
# {color:#ff8b00}TaskManagerTest{color} (owner: Christo)
# {color:#FF8B00}InternalTopicManagerTest{color} (owner: Christo)
# {color:#FF8B00}ProcessorContextImplTest{color} (owner: Christo)
# {color:#FF8B00}WriteConsistencyVectorTest{color} (owner: Christo)
# {color:#FF8B00}StreamsAssignmentScaleTest{color} (owner: Christo)
# {color:#FF8B00}StreamsPartitionAssignorTest{color} (owner: Christo)
# {color:#FF8B00}AssignmentTestUtils{color} (owner: Christo)
# {color:#FF8B00}ProcessorStateManagerTest{color} (owner: Matthew)
# {color:#FF8B00}StandbyTaskTest{color} (owner: Matthew)
# {color:#FF8B00}StoreChangelogReaderTest{color} (owner: Matthew)
# {color:#FF8B00}StreamTaskTest{color} (owner: Matthew)
# {color:#FF8B00}StreamThreadTest{color} (owner: Matthew)
# -StreamsMetricsImplTest (owner:- [~shekharrajak]{-}){-} (Captured in
https://issues.apache.org/jira/browse/KAFKA-12947)
# {color:#ff8b00}TimeOrderedCachingPersistentWindowStoreTest{color} (owner:
[~shekharrajak]) ([https://github.com/apache/kafka/pull/12739])
# TimeOrderedWindowStoreTest (owners: [~shekharrajak])
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProdu
[jira] [Updated] (KAFKA-14133) Remaining EasyMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14133?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14133:
--
Description:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (connect) (owner:
[~yash.mayya] )
# {color:#00875a}KStreamFlatTransformTest{color} (owner: Christo)
# {color:#00875a}KStreamFlatTransformValuesTest{color} (owner: Christo)
# {color:#00875a}KStreamPrintTest{color} (owner: Christo)
# {color:#00875a}KStreamRepartitionTest{color} (owner: Christo)
# {color:#00875a}MaterializedInternalTest{color} (owner: Christo)
# {color:#00875a}TransformerSupplierAdapterTest{color} (owner: Christo)
# {color:#00875a}KTableSuppressProcessorMetricsTest{color} (owner: Christo)
# {color:#00875a}ClientUtilsTest{color} (owner: Christo)
# {color:#00875a}HighAvailabilityStreamsPartitionAssignorTest{color} (owner:
Christo)
# {color:#00875A}TopologyTest{color} (owner: Christo)
# {color:#00875A}KTableSuppressProcessorTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingSessionBytesStoreTest{color} (owner: Christo)
# {color:#00875A}ChangeLoggingTimestampedWindowBytesStoreTest{color} (owner:
Christo)
# {color:#00875A}ChangeLoggingWindowBytesStoreTest{color} (owner: Christo)
# {color:#00875A}MeteredTimestampedWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamsRebalanceListenerTest{color} (owner: Christo)
# {color:#ff8b00}TimestampedKeyValueStoreMaterializerTest{color} (owner:
Christo)
# {color:#ff8b00}CachingInMemoryKeyValueStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingInMemorySessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentSessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingKeyValueBytesStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingTimestampedKeyValueBytesStoreTest{color} (owner:
Christo)
# {color:#ff8b00}CompositeReadOnlyWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}KeyValueStoreBuilderTest{color} (owner: Christo)
# {color:#ff8b00}RocksDBStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamThreadStateStoreProviderTest{color} (owner: Christo)
# {color:#ff8b00}TaskManagerTest{color} (owner: Christo)
# {color:#FF8B00}InternalTopicManagerTest{color} (owner: Christo)
# {color:#FF8B00}ProcessorContextImplTest{color} (owner: Christo)
# {color:#FF8B00}WriteConsistencyVectorTest{color} (owner: Christo)
# {color:#FF8B00}StreamsAssignmentScaleTest{color} (owner: Christo)
# {color:#FF8B00}StreamsPartitionAssignorTest{color} (owner: Christo)
# {color:#FF8B00}AssignmentTestUtils{color} (owner: Christo)
# ProcessorStateManagerTest ({*}WIP{*} owner: Matthew)
# StandbyTaskTest ({*}WIP{*} owner: Matthew)
# StoreChangelogReaderTest ({*}WIP{*} owner: Matthew)
# StreamTaskTest ({*}WIP{*} owner: Matthew)
# StreamThreadTest ({*}WIP{*} owner: Matthew)
# -StreamsMetricsImplTest ({*}WIP{*} owner:- [~shekharrajak]{-}){-} (Captured
in https://issues.apache.org/jira/browse/KAFKA-12947)
# {color:#ff8b00}TimeOrderedCachingPersistentWindowStoreTest{color} (owner:
[~shekharrajak]) ([https://github.com/apache/kafka/pull/12739])
# TimeOrderedWindowStoreTest
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (connect) (owner:
[~yash.mayya] )
# {color:#00875a}
[jira] [Updated] (KAFKA-14133) Remaining EasyMock to Mockito tests
[
https://issues.apache.org/jira/browse/KAFKA-14133?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christo Lolov updated KAFKA-14133:
--
Description:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (connect) (owner:
[~yash.mayya] )
# {color:#00875a}KStreamFlatTransformTest{color} (owner: Christo)
# {color:#00875a}KStreamFlatTransformValuesTest{color} (owner: Christo)
# {color:#00875a}KStreamPrintTest{color} (owner: Christo)
# {color:#00875a}KStreamRepartitionTest{color} (owner: Christo)
# {color:#00875a}MaterializedInternalTest{color} (owner: Christo)
# {color:#00875a}TransformerSupplierAdapterTest{color} (owner: Christo)
# {color:#00875a}KTableSuppressProcessorMetricsTest{color} (owner: Christo)
# {color:#00875a}ClientUtilsTest{color} (owner: Christo)
# {color:#00875a}HighAvailabilityStreamsPartitionAssignorTest{color} (owner:
Christo)
# {color:#ff8b00}StreamsRebalanceListenerTest{color} (owner: Christo)
# {color:#ff8b00}TimestampedKeyValueStoreMaterializerTest{color} (owner:
Christo)
# {color:#ff8b00}CachingInMemoryKeyValueStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingInMemorySessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentSessionStoreTest{color} (owner: Christo)
# {color:#ff8b00}CachingPersistentWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingKeyValueBytesStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingTimestampedKeyValueBytesStoreTest{color} (owner:
Christo)
# {color:#ff8b00}CompositeReadOnlyWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}KeyValueStoreBuilderTest{color} (owner: Christo)
# {color:#ff8b00}RocksDBStoreTest{color} (owner: Christo)
# {color:#ff8b00}StreamThreadStateStoreProviderTest{color} (owner: Christo)
# {color:#ff8b00}TopologyTest{color} (owner: Christo)
# {color:#ff8b00}KTableSuppressProcessorTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingSessionBytesStoreTest{color} (owner: Christo)
# {color:#ff8b00}ChangeLoggingTimestampedWindowBytesStoreTest{color} (owner:
Christo)
# {color:#ff8b00}ChangeLoggingWindowBytesStoreTest{color} (owner: Christo)
# {color:#ff8b00}MeteredTimestampedWindowStoreTest{color} (owner: Christo)
# {color:#ff8b00}TaskManagerTest{color} (owner: Christo)
# {color:#FF8B00}InternalTopicManagerTest{color} (owner: Christo)
# {color:#FF8B00}ProcessorContextImplTest{color} (owner: Christo)
# {color:#FF8B00}WriteConsistencyVectorTest{color} (owner: Christo)
# {color:#FF8B00}StreamsAssignmentScaleTest{color} (owner: Christo)
# {color:#FF8B00}StreamsPartitionAssignorTest{color} (owner: Christo)
# {color:#FF8B00}AssignmentTestUtils{color} (owner: Christo)
# ProcessorStateManagerTest ({*}WIP{*} owner: Matthew)
# StandbyTaskTest ({*}WIP{*} owner: Matthew)
# StoreChangelogReaderTest ({*}WIP{*} owner: Matthew)
# StreamTaskTest ({*}WIP{*} owner: Matthew)
# StreamThreadTest ({*}WIP{*} owner: Matthew)
# -StreamsMetricsImplTest ({*}WIP{*} owner:- [~shekharrajak]{-}){-} (Captured
in https://issues.apache.org/jira/browse/KAFKA-12947)
# {color:#ff8b00}TimeOrderedCachingPersistentWindowStoreTest{color} (owner:
[~shekharrajak]) ([https://github.com/apache/kafka/pull/12739])
# TimeOrderedWindowStoreTest
*The coverage report for the above tests after the change should be >= to what
the coverage is now.*
was:
{color:#de350b}There are tests which use both PowerMock and EasyMock. I have
put those in https://issues.apache.org/jira/browse/KAFKA-14132. Tests here rely
solely on EasyMock.{color}
Unless stated in brackets the tests are in the streams module.
A list of tests which still require to be moved from EasyMock to Mockito as of
2nd of August 2022 which do not have a Jira issue and do not have pull requests
I am aware of which are opened:
{color:#ff8b00}In Review{color}
{color:#00875a}Merged{color}
# {color:#00875a}WorkerConnectorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}WorkerCoordinatorTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}RootResourceTest{color} (connect) (owner: [~yash.mayya] )
# {color:#00875a}ByteArrayProducerRecordEquals{color} (connect) (owner:
[~yash.mayya] )
# {color:#00875a}
[GitHub] [kafka] clolov commented on pull request #12818: KAFKA-14133: Replace EasyMock with Mockito in streams tests
clolov commented on PR #12818: URL: https://github.com/apache/kafka/pull/12818#issuecomment-1302325278 @cadonna for visibility -- 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
[GitHub] [kafka] clolov commented on pull request #12818: KAFKA-14133: Replace EasyMock with Mockito in streams tests
clolov commented on PR #12818: URL: https://github.com/apache/kafka/pull/12818#issuecomment-1302324578 Three points for this pull request: * I have left `mock()` instead of `@Mock` where abstracting it wouldn't have proved easy. * I have used `lenient()` with some strict stubs because otherwise Mockito complains we do not use that code in some tests. I chose to not break the abstraction. * If I have deleted expectations that’s because Mockito said they are not used. -- 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
[GitHub] [kafka] clolov opened a new pull request, #12818: KAFKA-14133: Replace EasyMock with Mockito in streams tests
clolov opened a new pull request, #12818: URL: https://github.com/apache/kafka/pull/12818 Batch 6 of the tests detailed in https://issues.apache.org/jira/browse/KAFKA-14133 which use EasyMock and need to be moved to Mockito. -- 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
[jira] [Comment Edited] (KAFKA-14345) Flakey DynamicConnectionQuotaTest should use correct error bounds
[
https://issues.apache.org/jira/browse/KAFKA-14345?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17628388#comment-17628388
]
Chris Egerton edited comment on KAFKA-14345 at 11/3/22 3:38 PM:
[~gharris1727] is this related to KAFKA-8059 and/or KAFKA-12511? If so, would
you mind linking this ticket to one or both of them?
was (Author: chrisegerton):
[~gharris1727] is this related to KAFKA-8059 and/or KAFKA-12511? If so, would
you mind linking this ticket to them?
> Flakey DynamicConnectionQuotaTest should use correct error bounds
> -
>
> Key: KAFKA-14345
> URL: https://issues.apache.org/jira/browse/KAFKA-14345
> Project: Kafka
> Issue Type: Test
> Components: network
>Reporter: Greg Harris
>Assignee: Greg Harris
>Priority: Minor
>
> The DynamicConnectionQuotaTest is an integration test targeting the
> throttling behavior of listeners' accept thread. This test has been flaking
> out recently with errors such as the following:
> {noformat}
> Caused by: org.opentest4j.AssertionFailedError: Listener PLAINTEXT connection
> rate 14.558271396827829 must be below 14.399 ==> expected:
> but was:
> at
> app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
> at
> app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
> at app//org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
> at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
> at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
> at
> app//kafka.network.DynamicConnectionQuotaTest.verifyConnectionRate(DynamicConnectionQuotaTest.scala:412)
> at
> app//kafka.network.DynamicConnectionQuotaTest.$anonfun$testDynamicListenerConnectionCreationRateQuota$4(DynamicConnectionQuotaTest.scala:227){noformat}
> The test appears to be using a hard-coded error bound of 1.2f, which does not
> appear to be correct given the windowed algorithm. Instead of a hardcoded
> value, the bound should conform to the test execution to assert a more
> accurate bound.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (KAFKA-14345) Flakey DynamicConnectionQuotaTest should use correct error bounds
[
https://issues.apache.org/jira/browse/KAFKA-14345?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17628388#comment-17628388
]
Chris Egerton commented on KAFKA-14345:
---
[~gharris1727] is this related to KAFKA-8059 and/or KAFKA-12511? If so, would
you mind linking this ticket to them?
> Flakey DynamicConnectionQuotaTest should use correct error bounds
> -
>
> Key: KAFKA-14345
> URL: https://issues.apache.org/jira/browse/KAFKA-14345
> Project: Kafka
> Issue Type: Test
> Components: network
>Reporter: Greg Harris
>Assignee: Greg Harris
>Priority: Minor
>
> The DynamicConnectionQuotaTest is an integration test targeting the
> throttling behavior of listeners' accept thread. This test has been flaking
> out recently with errors such as the following:
> {noformat}
> Caused by: org.opentest4j.AssertionFailedError: Listener PLAINTEXT connection
> rate 14.558271396827829 must be below 14.399 ==> expected:
> but was:
> at
> app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
> at
> app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
> at app//org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
> at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
> at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
> at
> app//kafka.network.DynamicConnectionQuotaTest.verifyConnectionRate(DynamicConnectionQuotaTest.scala:412)
> at
> app//kafka.network.DynamicConnectionQuotaTest.$anonfun$testDynamicListenerConnectionCreationRateQuota$4(DynamicConnectionQuotaTest.scala:227){noformat}
> The test appears to be using a hard-coded error bound of 1.2f, which does not
> appear to be correct given the windowed algorithm. Instead of a hardcoded
> value, the bound should conform to the test execution to assert a more
> accurate bound.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [kafka] C0urante commented on a diff in pull request #12817: KAFKA-14346: Remove difficult to mock Plugins.compareAndSwapLoader usages
C0urante commented on code in PR #12817:
URL: https://github.com/apache/kafka/pull/12817#discussion_r1012953885
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##
@@ -147,6 +148,7 @@ public class AbstractHerderTest {
@Mock private ConfigBackingStore configStore;
@Mock private StatusBackingStore statusStore;
@Mock private ClassLoader classLoader;
+@Mock private LoaderSwap loaderSwap;
@Mock private Plugins plugins;
private ClassLoader loader;
Review Comment:
Can we remove the `@Before` and `@After` methods now that we know that these
tests aren't going to overwrite the context class loader?
##
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConnector.java:
##
@@ -116,14 +115,12 @@ public void run() {
LoggingContext.clear();
try (LoggingContext loggingContext =
LoggingContext.forConnector(connName)) {
-ClassLoader savedLoader = Plugins.compareAndSwapLoaders(loader);
Review Comment:
Would it also be reasonable to get rid of the `loader` constructor
parameter/field/getter method as well, since we no longer actually use that
field in this class?
##
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java:
##
@@ -1149,31 +1118,21 @@ public void setTargetState(String connName, TargetState
state, Callback workerConnector.transitionTo(state, stateChangeCallback),
-connectorLoader);
+try (LoaderSwap loaderSwap =
plugins.withClassLoader(workerConnector.loader())) {
+workerConnector.transitionTo(state, stateChangeCallback);
+}
}
for (Map.Entry taskEntry :
tasks.entrySet()) {
if (taskEntry.getKey().connector().equals(connName)) {
WorkerTask workerTask = taskEntry.getValue();
-executeStateTransition(() -> workerTask.transitionTo(state),
workerTask.loader);
+try (LoaderSwap loaderSwap =
plugins.withClassLoader(workerTask.loader())) {
+workerTask.transitionTo(state);
+}
}
}
}
-private void executeStateTransition(Runnable stateTransition, ClassLoader
loader) {
-ClassLoader savedLoader = plugins.currentThreadLoader();
-try {
-savedLoader = Plugins.compareAndSwapLoaders(loader);
-stateTransition.run();
-} finally {
-Plugins.compareAndSwapLoaders(savedLoader);
-}
-}
Review Comment:
Thanks for getting rid of this, it was a bit of a hack 👍
##
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java:
##
@@ -155,10 +143,22 @@ public LoaderSwap withClassLoader(ClassLoader loader) {
}
}
+public Runnable withClassLoader(ClassLoader classLoader, Runnable
operation) {
Review Comment:
Probably worth adding a Javadoc to this method. I initially thought it would
run the `operation` with the given loader, instead of returning a new
`Runnable` that wraps it.
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##
@@ -452,7 +454,8 @@ public void testConfigValidationMissingName() {
assertEquals(1, infos.get("required").configValue().errors().size());
verify(plugins).newConnector(connectorClass.getName());
-verify(plugins).compareAndSwapLoaders(connector);
+verify(plugins).withClassLoader(classLoader);
+verify(loaderSwap).close();
Review Comment:
Interesting--we never had coverage to ensure that we swapped back to the
original loader? Good to see that added now.
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java:
##
@@ -300,14 +302,14 @@ public void testStartAndStopConnector() throws Throwable {
connectorProps.put(CONNECTOR_CLASS_CONFIG, connectorClass);
// Create
-when(plugins.currentThreadLoader()).thenReturn(delegatingLoader);
-when(plugins.delegatingLoader()).thenReturn(delegatingLoader);
-
when(delegatingLoader.connectorLoader(connectorClass)).thenReturn(pluginLoader);
+when(plugins.connectorLoader(connectorClass)).thenReturn(pluginLoader);
when(plugins.newConnector(connectorClass)).thenReturn(sourceConnector);
when(sourceConnector.version()).thenReturn("1.0");
-pluginsMockedStatic.when(() ->
Plugins.compareAndSwapLoaders(pluginLoader)).thenReturn(delegatingLoader);
-pluginsMockedStatic.when(() ->
Plugins.compareAndSwapLoaders(delegatingLoader)).thenReturn(pluginLoader);
+when(plugins.withClassLoader(pluginLoader)).thenReturn(loaderSwap);
+// this test expects the runnable to be run by the executor, make
withClassLoader(cl, runnable) a passthrough.
+ArgumentCaptor runnableCaptor =
ArgumentCaptor.forClass
[GitHub] [kafka] cadonna commented on a diff in pull request #12795: KAFKA-14299: Initialize directly after handleAssignment
cadonna commented on code in PR #12795:
URL: https://github.com/apache/kafka/pull/12795#discussion_r1012934682
##
streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java:
##
@@ -395,8 +395,10 @@ private void createNewTasks(final Map> activeTasksTo
tasks.addActiveTasks(newActiveTasks);
tasks.addStandbyTasks(newStandbyTask);
} else {
-tasks.addPendingTaskToInit(newActiveTasks);
-tasks.addPendingTaskToInit(newStandbyTask);
+final Map taskInitExceptions = new
LinkedHashMap<>();
+Stream.concat(newActiveTasks.stream(), newStandbyTask.stream())
+.forEach(t -> addTaskToStateUpdater(t,
taskInitExceptions));
Review Comment:
Yeah, we considered also to initialise the tasks inside the state updater,
but we decided to do it outside so that we do not need to change the state of a
task inside the state updater. It seemed cleaner to us to keep lifecycle
management away from the state updater. The state updater should only update
the states.
What are the advantages of moving the initialisation into the state updater?
I see that we would not need to explicitly handle exceptions during
intialisation of tasks since that would happen automatically within the state
updater. Is there something else?
--
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
[GitHub] [kafka] lucasbru commented on a diff in pull request #12795: KAFKA-14299: Initialize directly after handleAssignment
lucasbru commented on code in PR #12795:
URL: https://github.com/apache/kafka/pull/12795#discussion_r1012833678
##
streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java:
##
@@ -395,8 +395,10 @@ private void createNewTasks(final Map> activeTasksTo
tasks.addActiveTasks(newActiveTasks);
tasks.addStandbyTasks(newStandbyTask);
} else {
-tasks.addPendingTaskToInit(newActiveTasks);
-tasks.addPendingTaskToInit(newStandbyTask);
+final Map taskInitExceptions = new
LinkedHashMap<>();
+Stream.concat(newActiveTasks.stream(), newStandbyTask.stream())
+.forEach(t -> addTaskToStateUpdater(t,
taskInitExceptions));
Review Comment:
If that is the case, especially if it takes a long time to init rocksdb,
maybe it would be the be easier to initialize the state in the state updater
instead?
##
streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java:
##
@@ -395,8 +395,10 @@ private void createNewTasks(final Map> activeTasksTo
tasks.addActiveTasks(newActiveTasks);
tasks.addStandbyTasks(newStandbyTask);
} else {
-tasks.addPendingTaskToInit(newActiveTasks);
-tasks.addPendingTaskToInit(newStandbyTask);
+final Map taskInitExceptions = new
LinkedHashMap<>();
+Stream.concat(newActiveTasks.stream(), newStandbyTask.stream())
+.forEach(t -> addTaskToStateUpdater(t,
taskInitExceptions));
Review Comment:
If that is the case, especially if it takes a long time to init rocksdb,
maybe it would be easier to initialize the state in the state updater instead?
--
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
[GitHub] [kafka] mimaison commented on a diff in pull request #12804: [Minior] [Test] KAFKA-14344 : Build EmbeddedKafkaCluster with common configs used for all clients
mimaison commented on code in PR #12804:
URL: https://github.com/apache/kafka/pull/12804#discussion_r1012744812
##
connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedKafkaCluster.java:
##
@@ -106,12 +106,20 @@ public class EmbeddedKafkaCluster {
private final String[] currentBrokerLogDirs;
private final boolean hasListenerConfig;
+final Map additionalClientConfigs;
Review Comment:
What about using `clientConfigs`? If you agree let's rename the setters too.
##
connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedKafkaCluster.java:
##
@@ -435,7 +445,9 @@ public Admin createAdminClient(Properties
adminClientConfig) {
adminClientConfig.put(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG,
((Password)
brokerConfig.get(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG)).value());
adminClientConfig.put(CommonClientConfigs.SECURITY_PROTOCOL_CONFIG, "SSL");
}
-return Admin.create(adminClientConfig);
+Properties finalAdminConfig =
Utils.mkProperties(additionalClientConfigs);
Review Comment:
In `createConsumer()` and `createProducer()` we add the custom client config
first, here we do it the other way around. Let use similar logic if we can
##
connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedKafkaCluster.java:
##
@@ -435,7 +445,9 @@ public Admin createAdminClient(Properties
adminClientConfig) {
adminClientConfig.put(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG,
((Password)
brokerConfig.get(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG)).value());
adminClientConfig.put(CommonClientConfigs.SECURITY_PROTOCOL_CONFIG, "SSL");
}
-return Admin.create(adminClientConfig);
+Properties finalAdminConfig =
Utils.mkProperties(additionalClientConfigs);
Review Comment:
I wonder if we could also change the type of `adminClientConfig` to `Map` so
it's similar to the other create methods.
--
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
[GitHub] [kafka] clolov commented on pull request #12809: [KAFKA-14324] Upgrade RocksDB to 7.1.2
clolov commented on PR #12809: URL: https://github.com/apache/kafka/pull/12809#issuecomment-1301915739 @cadonna, I wrote earlier today to https://groups.google.com/g/rocksdb/c/DWsH8Yda5gc. I will wait for a day and if there isn't a response I will also open an issue as you suggest :) -- 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
[GitHub] [kafka] OmniaGM commented on pull request #12577: KAFKA-13401: KIP-787 - MM2 manage Kafka resources with custom Admin implementation.
OmniaGM commented on PR #12577: URL: https://github.com/apache/kafka/pull/12577#issuecomment-1301870054 > Thanks for the updates @OmniaGM. There's a test failure: > > ``` > org.apache.kafka.common.KafkaException: The constructor of org.apache.kafka.clients.admin.ForwardingAdmin threw an exception >at app//org.apache.kafka.common.utils.Utils.newParameterizedInstance(Utils.java:466) >at app//org.apache.kafka.connect.mirror.MirrorClientConfig.forwardingAdmin(MirrorClientConfig.java:81) >at app//org.apache.kafka.connect.mirror.MirrorMakerConfigTest.testClientConfigProperties(MirrorMakerConfigTest.java:107) > ``` fixed now -- 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
[GitHub] [kafka] cadonna commented on pull request #12809: [KAFKA-14324] Upgrade RocksDB to 7.1.2
cadonna commented on PR #12809: URL: https://github.com/apache/kafka/pull/12809#issuecomment-1301816227 @clolov, to get option 1 rolling I think you could open an issue at https://github.com/facebook/rocksdb/issues and ask if it is possible to get a patch release of 6.29 with the zlib CVE fixed. Please also explain that for Kafka Streams it is hard to move to 7.1.2 due to backwards compatibility issues. Let me know if you need any help from our side for this. -- 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
[GitHub] [kafka] dajac commented on a diff in pull request #12783: KAFKA-14334: complete delayed purgatory after replication
dajac commented on code in PR #12783:
URL: https://github.com/apache/kafka/pull/12783#discussion_r1012620903
##
core/src/main/scala/kafka/log/UnifiedLog.scala:
##
@@ -433,6 +433,22 @@ class UnifiedLog(@volatile var logStartOffset: Long,
}
}
+ /**
Review Comment:
nit: Could we also update the scaladoc of `def updateHighWatermark(hw:
Long)` to remove the part about the follower updating the HWM. This is no
longer the case.
##
core/src/test/scala/integration/kafka/server/FetchFromFollowerIntegrationTest.scala:
##
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package integration.kafka.server
+
+import kafka.server.{BaseFetchRequestTest, KafkaConfig}
+import kafka.utils.{TestInfoUtils, TestUtils}
+import org.apache.kafka.clients.admin.Admin
+import org.apache.kafka.common.TopicPartition
+import org.apache.kafka.common.protocol.{ApiKeys, Errors}
+import org.apache.kafka.common.requests.FetchResponse
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.{BeforeEach, Timeout}
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
+
+import java.util.Properties
+import scala.jdk.CollectionConverters._
+
+class FetchFromFollowerIntegrationTest extends BaseFetchRequestTest {
+ val numNodes = 2
+ val numParts = 1
+
+ val topic = "test-fetch-from-follower"
+ val leaderBrokerId = 0
+ val followerBrokerId = 1
+ var admin: Admin = null
+
+ def overridingProps: Properties = {
+val props = new Properties
+props.put(KafkaConfig.NumPartitionsProp, numParts.toString)
+props
+ }
+
+ override def generateConfigs: collection.Seq[KafkaConfig] = {
+TestUtils.createBrokerConfigs(numNodes, zkConnectOrNull,
enableControlledShutdown = false, enableFetchFromFollower = true)
+ .map(KafkaConfig.fromProps(_, overridingProps))
+ }
+
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
+ @ValueSource(strings = Array("zk", "kraft"))
+ @Timeout(15)
+ def testFollowerCompleteDelayedFetchesOnReplication(quorum: String): Unit = {
+// Create a 2 broker cluster where broker 0 is the leader and 1 is the
follower.
+admin = createAdminClient()
+TestUtils.createTopicWithAdmin(
+ admin,
+ topic,
+ brokers,
+ replicaAssignment = Map(0 -> Seq(leaderBrokerId, followerBrokerId))
+)
+
+// Set fetch.max.wait.ms to a value (20 seconds) greater than the timeout
(15 seconds).
+// Send a fetch request before the record is replicated to ensure that the
replication
+// triggers purgatory completion.
Review Comment:
nit: Should we move this to right before `val fetchRequest` as the comment
is for this part?
##
core/src/main/scala/kafka/server/ReplicaFetcherThread.scala:
##
@@ -132,9 +139,22 @@ class ReplicaFetcherThread(name: String,
brokerTopicStats.updateReplicationBytesIn(records.sizeInBytes)
+val highWatermarkChanged =
log.maybeUpdateHighWatermark(partitionData.highWatermark)
+if (highWatermarkChanged) {
+ logAppendInfo.foreach { _ => partitionsWithNewHighWatermark +=
topicPartition }
+ if (logTrace)
+trace(s"Follower updated replica high watermark for partition
$topicPartition to ${partitionData.highWatermark}")
Review Comment:
I see. How about returning an `Option` in `maybeUpdateHighWatermark` which
would contain the updated HWM only if it was changed. Would this work?
I would also update the trace message to include both the received HWM from
the leader and the updated HWM. Both seems useful here.
##
core/src/test/scala/integration/kafka/server/FetchFromFollowerIntegrationTest.scala:
##
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/
