From a059d85539e922215ab15ceb772c2bd2513a2041 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 19 Sep 2022 14:12:05 -0700 Subject: [PATCH 01/45] Basic event handler definition --- .../internals/DefaultEventHandler.java | 42 ++++++------------- .../consumer/internals/EventHandler.java | 23 ++++++++++ .../events/ConsumerRequestEvent.java | 4 ++ .../events/ConsumerResponseEvent.java | 4 ++ 4 files changed, 44 insertions(+), 29 deletions(-) create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index ec1368ed801f8..26ad69c473cc1 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -16,45 +16,29 @@ */ package org.apache.kafka.clients.consumer.internals; -import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; -import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; -import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.clients.consumer.internals.events.ConsumerRequestEvent; +import org.apache.kafka.clients.consumer.internals.events.ConsumerResponseEvent; -import java.util.Optional; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; -/** - * This class interfaces the KafkaConsumer and the background thread. It allows the caller to enqueue {@link ApplicationEvent} - * to be consumed by the background thread and poll {@linkBackgroundEvent} produced by the background thread. - */ -public class DefaultEventHandler implements EventHandler { - private final BlockingQueue applicationEvents; - private final BlockingQueue backgroundEvents; - - public DefaultEventHandler() { - this.applicationEvents = new LinkedBlockingQueue<>(); - this.backgroundEvents = new LinkedBlockingQueue<>(); - // TODO: a concreted implementation of how requests are being consumed, and how responses are being produced. - } +public class DefaultEventHandler implements EventHandler { + BlockingQueue consumerRequestEvents; + BlockingQueue consumerResponseEvents; - @Override - public Optional poll() { - return Optional.ofNullable(backgroundEvents.poll()); + public DefaultEventHandler(ConsumerConfig config) { + this.consumerRequestEvents = new LinkedBlockingQueue<>(); + this.consumerResponseEvents = new LinkedBlockingQueue<>(); } @Override - public boolean isEmpty() { - return backgroundEvents.isEmpty(); + public ConsumerResponseEvent poll() { + return consumerResponseEvents.poll(); } @Override - public boolean add(ApplicationEvent event) { - try { - return applicationEvents.add(event); - } catch (IllegalStateException e) { - // swallow the capacity restriction exception - return false; - } + public boolean add(ConsumerRequestEvent event) { + return consumerRequestEvents.add(event); } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java new file mode 100644 index 0000000000000..12e9171b2851c --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java @@ -0,0 +1,23 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals; + +public interface EventHandler { + public K poll(); + public boolean add(T event); +} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java new file mode 100644 index 0000000000000..bfb4a4de693f8 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java @@ -0,0 +1,4 @@ +package org.apache.kafka.clients.consumer.internals.events; + +public interface ConsumerRequestEvent { +} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java new file mode 100644 index 0000000000000..4aa36182c1323 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java @@ -0,0 +1,4 @@ +package org.apache.kafka.clients.consumer.internals.events; + +public interface ConsumerResponseEvent { +} From 008ca46c1301a61e5708e860b48c10cd3ac14877 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 19 Sep 2022 14:26:23 -0700 Subject: [PATCH 02/45] Define EventHandler interface --- .../internals/DefaultEventHandler.java | 5 ++-- .../consumer/internals/EventHandler.java | 23 ------------------- .../events/ConsumerRequestEvent.java | 19 ++++++++++++++- .../events/ConsumerResponseEvent.java | 19 ++++++++++++++- .../internals/events/EventHandler.java | 1 - 5 files changed, 39 insertions(+), 28 deletions(-) delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 26ad69c473cc1..cb94822ba3acc 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -16,9 +16,9 @@ */ package org.apache.kafka.clients.consumer.internals; -import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.clients.consumer.internals.events.ConsumerRequestEvent; import org.apache.kafka.clients.consumer.internals.events.ConsumerResponseEvent; +import org.apache.kafka.clients.consumer.internals.events.EventHandler; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -27,9 +27,10 @@ public class DefaultEventHandler implements EventHandler consumerRequestEvents; BlockingQueue consumerResponseEvents; - public DefaultEventHandler(ConsumerConfig config) { + public DefaultEventHandler() { this.consumerRequestEvents = new LinkedBlockingQueue<>(); this.consumerResponseEvents = new LinkedBlockingQueue<>(); + // TODO: a concreted implementation of how requests are being consumed, and how responses are being produced. } @Override diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java deleted file mode 100644 index 12e9171b2851c..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * 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 org.apache.kafka.clients.consumer.internals; - -public interface EventHandler { - public K poll(); - public boolean add(T event); -} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java index bfb4a4de693f8..9df5e37a57f9e 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java @@ -1,4 +1,21 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals.events; -public interface ConsumerRequestEvent { +abstract public class ConsumerRequestEvent { } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java index 4aa36182c1323..7621d0f42f2c7 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java @@ -1,4 +1,21 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals.events; -public interface ConsumerResponseEvent { +abstract public class ConsumerResponseEvent { } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java index d9f5b9d065bbc..c07cc33cf7353 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java @@ -14,7 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.apache.kafka.clients.consumer.internals.events; import java.util.Optional; From 5bb33622878b693e4e9a7fb79a3fa715023fc8db Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 19 Sep 2022 14:42:32 -0700 Subject: [PATCH 03/45] lint error --- .../clients/consumer/internals/events/ConsumerRequestEvent.java | 1 - .../clients/consumer/internals/events/ConsumerResponseEvent.java | 1 - 2 files changed, 2 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java index 9df5e37a57f9e..16122981c744e 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java @@ -13,7 +13,6 @@ * 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 org.apache.kafka.clients.consumer.internals.events; diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java index 7621d0f42f2c7..e19d8d2dcef30 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java @@ -13,7 +13,6 @@ * 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 org.apache.kafka.clients.consumer.internals.events; From 655180749ac9ef6f00771c336442788e4ed1eb6c Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 19 Sep 2022 16:46:39 -0700 Subject: [PATCH 04/45] moving packages --- .../kafka/clients/consumer/internals/DefaultEventHandler.java | 1 - .../clients/consumer/internals/{events => }/EventHandler.java | 0 2 files changed, 1 deletion(-) rename clients/src/main/java/org/apache/kafka/clients/consumer/internals/{events => }/EventHandler.java (100%) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index cb94822ba3acc..29930c4b71886 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -18,7 +18,6 @@ import org.apache.kafka.clients.consumer.internals.events.ConsumerRequestEvent; import org.apache.kafka.clients.consumer.internals.events.ConsumerResponseEvent; -import org.apache.kafka.clients.consumer.internals.events.EventHandler; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java similarity index 100% rename from clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java rename to clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java From 86191c259b3ac46f10b9033b77c93bbf3bacaf32 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 19 Sep 2022 17:11:22 -0700 Subject: [PATCH 05/45] reduce the scope --- .../kafka/clients/consumer/internals/DefaultEventHandler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 29930c4b71886..baf789723bb1e 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -23,8 +23,8 @@ import java.util.concurrent.LinkedBlockingQueue; public class DefaultEventHandler implements EventHandler { - BlockingQueue consumerRequestEvents; - BlockingQueue consumerResponseEvents; + private BlockingQueue consumerRequestEvents; + private BlockingQueue consumerResponseEvents; public DefaultEventHandler() { this.consumerRequestEvents = new LinkedBlockingQueue<>(); From c0ef14352710655bfffc974253628a1d3dea1117 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 21 Sep 2022 09:54:04 -0700 Subject: [PATCH 06/45] PR comments for naming and documentation --- .../internals/DefaultEventHandler.java | 27 +++++++++++-------- .../events/ConsumerRequestEvent.java | 20 -------------- .../events/ConsumerResponseEvent.java | 20 -------------- .../internals/{ => events}/EventHandler.java | 0 4 files changed, 16 insertions(+), 51 deletions(-) delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java rename clients/src/main/java/org/apache/kafka/clients/consumer/internals/{ => events}/EventHandler.java (100%) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index baf789723bb1e..c6014d43679dc 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -16,29 +16,34 @@ */ package org.apache.kafka.clients.consumer.internals; -import org.apache.kafka.clients.consumer.internals.events.ConsumerRequestEvent; -import org.apache.kafka.clients.consumer.internals.events.ConsumerResponseEvent; +import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; +import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; +import org.apache.kafka.clients.consumer.internals.events.EventHandler; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; -public class DefaultEventHandler implements EventHandler { - private BlockingQueue consumerRequestEvents; - private BlockingQueue consumerResponseEvents; +/** + * This class interfaces the KafkaConsumer and the background thread. It allows the caller to enqueue {@link ApplicationEvent} + * to be consumed by the background thread and poll {@linkBackgroundEvent} produced by the background thread. + */ +public class DefaultEventHandler implements EventHandler { + private BlockingQueue applicationEvents; + private BlockingQueue backgroundEvents; public DefaultEventHandler() { - this.consumerRequestEvents = new LinkedBlockingQueue<>(); - this.consumerResponseEvents = new LinkedBlockingQueue<>(); + this.applicationEvents = new LinkedBlockingQueue<>(); + this.backgroundEvents = new LinkedBlockingQueue<>(); // TODO: a concreted implementation of how requests are being consumed, and how responses are being produced. } @Override - public ConsumerResponseEvent poll() { - return consumerResponseEvents.poll(); + public BackgroundEvent poll() { + return backgroundEvents.poll(); } @Override - public boolean add(ConsumerRequestEvent event) { - return consumerRequestEvents.add(event); + public boolean add(ApplicationEvent event) { + return applicationEvents.add(event); } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java deleted file mode 100644 index 16122981c744e..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * 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 org.apache.kafka.clients.consumer.internals.events; - -abstract public class ConsumerRequestEvent { -} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java deleted file mode 100644 index e19d8d2dcef30..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerResponseEvent.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * 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 org.apache.kafka.clients.consumer.internals.events; - -abstract public class ConsumerResponseEvent { -} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java similarity index 100% rename from clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java rename to clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java From 564660f8ac1c712a3bad87a7573e95b7cd16773d Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 21 Sep 2022 12:21:09 -0700 Subject: [PATCH 07/45] Documentation on the beahvior Documentation on the beahvior --- .../clients/consumer/internals/DefaultEventHandler.java | 5 +++-- .../clients/consumer/internals/events/EventHandler.java | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index c6014d43679dc..611cfe8d7b1dc 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -20,6 +20,7 @@ import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import java.util.Optional; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -38,8 +39,8 @@ public DefaultEventHandler() { } @Override - public BackgroundEvent poll() { - return backgroundEvents.poll(); + public Optional poll() { + return Optional.ofNullable(backgroundEvents.poll()); } @Override diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java index c07cc33cf7353..4e96dc0579e76 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java @@ -39,7 +39,7 @@ public interface EventHandler { * Add an {@link ApplicationEvent} to the handler. The method returns true upon successful add; otherwise returns * false. * @param event An {@link ApplicationEvent} created by the polling thread. - * @return true upon successful add. + * @return {@code true} upon successful add, {@code false} otherwise. */ - boolean add(ApplicationEvent event); + public boolean add(ApplicationEvent event); } From 4f0a15ec359b7ac0476909f16c4547d1fdbd4fef Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 21 Sep 2022 13:09:10 -0700 Subject: [PATCH 08/45] Add isEmpty and remove excessive public --- .../consumer/internals/DefaultEventHandler.java | 10 ++++++++-- .../consumer/internals/events/EventHandler.java | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 611cfe8d7b1dc..0beb62c89d63e 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -19,6 +19,7 @@ import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import org.apache.kafka.common.errors.InterruptException; import java.util.Optional; import java.util.concurrent.BlockingQueue; @@ -29,8 +30,8 @@ * to be consumed by the background thread and poll {@linkBackgroundEvent} produced by the background thread. */ public class DefaultEventHandler implements EventHandler { - private BlockingQueue applicationEvents; - private BlockingQueue backgroundEvents; + private final BlockingQueue applicationEvents; + private final BlockingQueue backgroundEvents; public DefaultEventHandler() { this.applicationEvents = new LinkedBlockingQueue<>(); @@ -43,6 +44,11 @@ public Optional poll() { return Optional.ofNullable(backgroundEvents.poll()); } + @Override + public boolean isEmpty() { + return backgroundEvents.isEmpty(); + } + @Override public boolean add(ApplicationEvent event) { return applicationEvents.add(event); diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java index 4e96dc0579e76..99812525faa94 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java @@ -41,5 +41,7 @@ public interface EventHandler { * @param event An {@link ApplicationEvent} created by the polling thread. * @return {@code true} upon successful add, {@code false} otherwise. */ - public boolean add(ApplicationEvent event); + boolean add(ApplicationEvent event); + + } From 4828f719d399e1a90ef805aa2aa2ca1da5d9a7bb Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 21 Sep 2022 13:25:01 -0700 Subject: [PATCH 09/45] Remove unused import --- .../kafka/clients/consumer/internals/DefaultEventHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 0beb62c89d63e..45d6d24ff2c93 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -19,7 +19,6 @@ import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; -import org.apache.kafka.common.errors.InterruptException; import java.util.Optional; import java.util.concurrent.BlockingQueue; From 779c00c674cdb3ed3a8e33537893857cbbf839df Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 21 Sep 2022 13:40:13 -0700 Subject: [PATCH 10/45] Handle capacity limitation --- .../clients/consumer/internals/DefaultEventHandler.java | 7 ++++++- .../clients/consumer/internals/events/EventHandler.java | 2 -- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 45d6d24ff2c93..ec1368ed801f8 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -50,6 +50,11 @@ public boolean isEmpty() { @Override public boolean add(ApplicationEvent event) { - return applicationEvents.add(event); + try { + return applicationEvents.add(event); + } catch (IllegalStateException e) { + // swallow the capacity restriction exception + return false; + } } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java index 99812525faa94..9a8dad84bce24 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java @@ -42,6 +42,4 @@ public interface EventHandler { * @return {@code true} upon successful add, {@code false} otherwise. */ boolean add(ApplicationEvent event); - - } From b18098d21d66ce1803faa90a6f82cefdc9619739 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 21 Sep 2022 19:36:57 -0700 Subject: [PATCH 11/45] clean up a comment --- .../kafka/clients/consumer/internals/events/EventHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java index 9a8dad84bce24..c07cc33cf7353 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java @@ -39,7 +39,7 @@ public interface EventHandler { * Add an {@link ApplicationEvent} to the handler. The method returns true upon successful add; otherwise returns * false. * @param event An {@link ApplicationEvent} created by the polling thread. - * @return {@code true} upon successful add, {@code false} otherwise. + * @return true upon successful add. */ boolean add(ApplicationEvent event); } From ce00e48eea271a78f4122b0587ff8286f4d01f26 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 26 Sep 2022 15:44:01 -0700 Subject: [PATCH 12/45] A stubbed event handler to demonstrate the usage. Stubbed event handler --- .../internals/events/NoopEventHandler.java | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java new file mode 100644 index 0000000000000..d7c2ca3d4b2d1 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java @@ -0,0 +1,109 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals.events; + +import org.apache.kafka.common.errors.InterruptException; +import org.apache.kafka.common.utils.KafkaThread; +import org.apache.kafka.common.utils.LogContext; +import org.slf4j.Logger; + +import java.util.Optional; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; + +/** + * The NoopEventHandler uses a background thread to process events in the ApplicationEventQueue. The background thread + * performs two simple tasks. First, it polls ApplicationEvents off the queue and logs a message for each event it + * consumes. Second, it handles the exception by sending an ExceptionBackgroundEvent to the backgroundEventQueue. + */ +public class NoopEventHandler implements EventHandler { + private final Logger log; + private final BlockingQueue applicationEventQueue; + private final BlockingQueue backgroundEventQueue; + private final Thread backgroundThread; + private Runnable noopProcessor; + + public NoopEventHandler() { + LogContext logContext = new LogContext("stubbed_event_handler"); + this.log = logContext.logger(NoopEventHandler.class); + this.applicationEventQueue = new LinkedBlockingQueue<>(); + this.backgroundEventQueue = new LinkedBlockingQueue<>(); + this.noopProcessor = new NoopProcessor(this.applicationEventQueue, this.backgroundEventQueue); + this.backgroundThread = new KafkaThread("stubbed_backgroundThread", this.noopProcessor, true); + backgroundThread.start(); + } + + @Override + public Optional poll() { + return Optional.ofNullable(backgroundEventQueue.poll()); + } + + @Override + public boolean isEmpty() { + return backgroundEventQueue.isEmpty(); + } + + @Override + public boolean add(ApplicationEvent event) { + return applicationEventQueue.add(event); + } + + private class NoopProcessor implements Runnable { + private static final long RETRY_BACKOFF_MS = 100; + private final BlockingQueue applicationEventQueue; + private final BlockingQueue backgroundEventQueue; + private volatile boolean running; + + NoopProcessor(BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue) { + this.applicationEventQueue = applicationEventQueue; + this.backgroundEventQueue = backgroundEventQueue; + this.running = true; + } + + @Override + public void run() { + while (running) { + try { + if (!applicationEventQueue.isEmpty()) { + ApplicationEvent event = applicationEventQueue.poll(); + processEvent(event); + } + this.wait(RETRY_BACKOFF_MS); + } catch (InterruptException e) { + Thread.interrupted(); + log.error("unexpected interruption", e); + } catch (Exception e) { + backgroundEventQueue.add(new ExceptionBackgroundEvent(e)); + log.error("Exception while processing events", e); + } + } + } + + private void processEvent(ApplicationEvent event) { + log.info("processing event: " + event); + } + } + + static class ExceptionBackgroundEvent extends BackgroundEvent { + public final static String EVENT_TYPE = "exception"; + public final Exception exception; + public ExceptionBackgroundEvent(Exception exception) { + this.exception = exception; + } + } +} From 30e059bab645f13a0b2eb1c45c9f0cc855391b19 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 26 Sep 2022 16:04:24 -0700 Subject: [PATCH 13/45] typo --- .../clients/consumer/internals/events/NoopEventHandler.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java index d7c2ca3d4b2d1..056c98d40fac0 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java @@ -28,7 +28,8 @@ /** * The NoopEventHandler uses a background thread to process events in the ApplicationEventQueue. The background thread * performs two simple tasks. First, it polls ApplicationEvents off the queue and logs a message for each event it - * consumes. Second, it handles the exception by sending an ExceptionBackgroundEvent to the backgroundEventQueue. + * consumes. Second, if it encounters an exception, the background thread will create an ExceptionBackgroundEvent + * and send it to the backgroundEventQueue. */ public class NoopEventHandler implements EventHandler { private final Logger log; From 0e5a6dd0c39557c3a119c708e23907272dd61086 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 27 Sep 2022 10:22:26 -0700 Subject: [PATCH 14/45] Revert "typo" This reverts commit 625b16c7acf42ccced1ca86cfcbb054b99fa7e59. --- .../clients/consumer/internals/events/NoopEventHandler.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java index 056c98d40fac0..d7c2ca3d4b2d1 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java @@ -28,8 +28,7 @@ /** * The NoopEventHandler uses a background thread to process events in the ApplicationEventQueue. The background thread * performs two simple tasks. First, it polls ApplicationEvents off the queue and logs a message for each event it - * consumes. Second, if it encounters an exception, the background thread will create an ExceptionBackgroundEvent - * and send it to the backgroundEventQueue. + * consumes. Second, it handles the exception by sending an ExceptionBackgroundEvent to the backgroundEventQueue. */ public class NoopEventHandler implements EventHandler { private final Logger log; From 1ed68361344aeaecfade94881d52a6e1d657a9d9 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 27 Sep 2022 10:22:31 -0700 Subject: [PATCH 15/45] Revert "A stubbed event handler to demonstrate the usage." This reverts commit 816b285b6e1a5112be8ce97a000b9a2a14bf7020. --- .../internals/events/NoopEventHandler.java | 109 ------------------ 1 file changed, 109 deletions(-) delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java deleted file mode 100644 index d7c2ca3d4b2d1..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopEventHandler.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * 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 org.apache.kafka.clients.consumer.internals.events; - -import org.apache.kafka.common.errors.InterruptException; -import org.apache.kafka.common.utils.KafkaThread; -import org.apache.kafka.common.utils.LogContext; -import org.slf4j.Logger; - -import java.util.Optional; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; - -/** - * The NoopEventHandler uses a background thread to process events in the ApplicationEventQueue. The background thread - * performs two simple tasks. First, it polls ApplicationEvents off the queue and logs a message for each event it - * consumes. Second, it handles the exception by sending an ExceptionBackgroundEvent to the backgroundEventQueue. - */ -public class NoopEventHandler implements EventHandler { - private final Logger log; - private final BlockingQueue applicationEventQueue; - private final BlockingQueue backgroundEventQueue; - private final Thread backgroundThread; - private Runnable noopProcessor; - - public NoopEventHandler() { - LogContext logContext = new LogContext("stubbed_event_handler"); - this.log = logContext.logger(NoopEventHandler.class); - this.applicationEventQueue = new LinkedBlockingQueue<>(); - this.backgroundEventQueue = new LinkedBlockingQueue<>(); - this.noopProcessor = new NoopProcessor(this.applicationEventQueue, this.backgroundEventQueue); - this.backgroundThread = new KafkaThread("stubbed_backgroundThread", this.noopProcessor, true); - backgroundThread.start(); - } - - @Override - public Optional poll() { - return Optional.ofNullable(backgroundEventQueue.poll()); - } - - @Override - public boolean isEmpty() { - return backgroundEventQueue.isEmpty(); - } - - @Override - public boolean add(ApplicationEvent event) { - return applicationEventQueue.add(event); - } - - private class NoopProcessor implements Runnable { - private static final long RETRY_BACKOFF_MS = 100; - private final BlockingQueue applicationEventQueue; - private final BlockingQueue backgroundEventQueue; - private volatile boolean running; - - NoopProcessor(BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue) { - this.applicationEventQueue = applicationEventQueue; - this.backgroundEventQueue = backgroundEventQueue; - this.running = true; - } - - @Override - public void run() { - while (running) { - try { - if (!applicationEventQueue.isEmpty()) { - ApplicationEvent event = applicationEventQueue.poll(); - processEvent(event); - } - this.wait(RETRY_BACKOFF_MS); - } catch (InterruptException e) { - Thread.interrupted(); - log.error("unexpected interruption", e); - } catch (Exception e) { - backgroundEventQueue.add(new ExceptionBackgroundEvent(e)); - log.error("Exception while processing events", e); - } - } - } - - private void processEvent(ApplicationEvent event) { - log.info("processing event: " + event); - } - } - - static class ExceptionBackgroundEvent extends BackgroundEvent { - public final static String EVENT_TYPE = "exception"; - public final Exception exception; - public ExceptionBackgroundEvent(Exception exception) { - this.exception = exception; - } - } -} From bbabb93db0f2f1c3b5bd7b6687c98755832b2581 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 27 Sep 2022 10:43:11 -0700 Subject: [PATCH 16/45] Prototyping --- .../AbstractPrototypeAsyncConsumer.java | 41 +++++++++++++++++++ .../events/CommitApplicationEvent.java | 6 +++ 2 files changed, 47 insertions(+) create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java new file mode 100644 index 0000000000000..df6b3a712e6a6 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java @@ -0,0 +1,41 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals; + +import org.apache.kafka.clients.consumer.Consumer; +import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; +import org.apache.kafka.clients.consumer.internals.events.EventHandler; + +/** + * This is the prototype of the consumer that uses the {@link EventHandler} to process application event. + */ +public abstract class AbstractPrototypeAsyncConsumer implements Consumer { + private final EventHandler eventHandler; + + public AbstractPrototypeAsyncConsumer(final EventHandler eventHandler) { + this.eventHandler = eventHandler; + } + + public void commitAsync() { + ApplicationEvent commitEvent = new CommitApplicationEvent(); + eventHandler.add(commitEvent); + } + + private static class CommitApplicationEvent extends ApplicationEvent { + // this is stubbed commitAsyncEvents + } +} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java new file mode 100644 index 0000000000000..1da02d35a1931 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java @@ -0,0 +1,6 @@ +package org.apache.kafka.clients.consumer.internals.events; +/** + * + */ +public class CommitApplicationEvent extends ApplicationEvent { +} From e081c5fc5ef1780f94c0555ff75b341a7c69fb75 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 27 Sep 2022 10:45:12 -0700 Subject: [PATCH 17/45] clean up and documentation clean up documentation implemented poll and commitSync clean up clean up --- .../AbstractPrototypeAsyncConsumer.java | 88 +++++++++++++++++-- .../events/CommitApplicationEvent.java | 6 -- 2 files changed, 83 insertions(+), 11 deletions(-) delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java index df6b3a712e6a6..79f7979ecbb8c 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java @@ -17,25 +17,103 @@ package org.apache.kafka.clients.consumer.internals; import org.apache.kafka.clients.consumer.Consumer; +import org.apache.kafka.clients.consumer.ConsumerRecords; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; +import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import org.apache.kafka.common.utils.Time; + +import java.time.Duration; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; /** - * This is the prototype of the consumer that uses the {@link EventHandler} to process application event. + * This is the prototype of the consumer that uses the {@link EventHandler} to process application events. */ -public abstract class AbstractPrototypeAsyncConsumer implements Consumer { +public abstract class AbstractPrototypeAsyncConsumer implements Consumer { private final EventHandler eventHandler; + private final Time time; - public AbstractPrototypeAsyncConsumer(final EventHandler eventHandler) { + public AbstractPrototypeAsyncConsumer(final Time time, final EventHandler eventHandler) { this.eventHandler = eventHandler; + this.time = time; + } + + /** + * poll implementation using {@link EventHandler}. + * 1. Poll for background events. If there's a fetch response event, process the record and return it. If it is + * another type of event, process it. + * 2. Send fetches if needed. + * If the timeout expires, return an empty ConsumerRecord. + * @param timeout timeout of the poll loop + * @return ConsumerRecord. It can be empty if time timeout expires. + */ + @Override + public ConsumerRecords poll(final Duration timeout) { + try { + do { + if (!eventHandler.isEmpty()) { + Optional backgroundEvent = eventHandler.poll(); + if (backgroundEvent.isPresent()) { + if (isFetchResult(backgroundEvent.get())) { + // return fetches + return processFetchResult(backgroundEvent.get()); + } + processEvent(backgroundEvent.get(), timeout); // might trigger callbacks or handle exceptions + } + } + + maybeSendFetches(); // send new fetches + } while (time.timer(timeout).notExpired()); + } catch (Exception e) { + throw new RuntimeException(e); + } + + return ConsumerRecords.empty(); } + abstract void processEvent(BackgroundEvent backgroundEvent, Duration timeout); + abstract boolean isFetchResult(BackgroundEvent event); + abstract ConsumerRecords processFetchResult(BackgroundEvent event); + abstract void maybeSendFetches(); + + /** + * This method sends a commit event to the EventHandler and return. + */ + @Override public void commitAsync() { ApplicationEvent commitEvent = new CommitApplicationEvent(); eventHandler.add(commitEvent); } - private static class CommitApplicationEvent extends ApplicationEvent { - // this is stubbed commitAsyncEvents + /** + * This method sends a commit event to the EventHandler and waits for the event to finish. + * @param timeout max wait time for the blocking operation. + */ + @Override + public void commitSync(Duration timeout) { + CommitApplicationEvent commitEvent = new CommitApplicationEvent(); + eventHandler.add(commitEvent); + + CompletableFuture commitFuture = commitEvent.commitFuture; + try { + commitFuture.get(timeout.toMillis(), TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + throw new org.apache.kafka.common.errors.TimeoutException("timeout"); + } catch (Exception e) { + // handle exception here + throw new RuntimeException(e); + } + } + + /** + * A stubbed ApplicationEvent for demonstration purpose + */ + private class CommitApplicationEvent extends ApplicationEvent { + // this is the stubbed commitAsyncEvents + CompletableFuture commitFuture = new CompletableFuture<>(); } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java deleted file mode 100644 index 1da02d35a1931..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java +++ /dev/null @@ -1,6 +0,0 @@ -package org.apache.kafka.clients.consumer.internals.events; -/** - * - */ -public class CommitApplicationEvent extends ApplicationEvent { -} From d5c8009a2f61611d3417621f9679b749e0fbdcc9 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 19 Sep 2022 14:35:57 -0700 Subject: [PATCH 18/45] background thread background thread --- .../AbstractPrototypeAsyncConsumer.java | 3 + .../internals/ConsumerBackgroundThread.java | 196 ++++++++++++++++++ .../internals/events/ApplicationEvent.java | 12 ++ 3 files changed, 211 insertions(+) create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java index 79f7979ecbb8c..f32fe4d74edf6 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java @@ -113,6 +113,9 @@ public void commitSync(Duration timeout) { * A stubbed ApplicationEvent for demonstration purpose */ private class CommitApplicationEvent extends ApplicationEvent { + public CommitApplicationEvent() { + super(EventTypes.COMMIT, false); + } // this is the stubbed commitAsyncEvents CompletableFuture commitFuture = new CompletableFuture<>(); } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java new file mode 100644 index 0000000000000..1ad53b6a74da6 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java @@ -0,0 +1,196 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals; + +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; +import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; +import org.apache.kafka.common.utils.KafkaThread; +import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.Time; +import org.slf4j.Logger; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.BlockingQueue; + +/** + * The background thread runs in the background and consumes the {@code ApplicationEvent} from the {@link org.apache.kafka.clients.consumer.KafkaConsumer} APIs. This class uses an event loop to drive the following important tasks: + *
    + *
  • Consuming and executing the {@code ApplicationEvent}.
  • + *
  • Maintaining the connection to the coordinator.
  • + *
  • Sending heartbeat.
  • + *
  • Autocommitting.
  • + *
  • Executing the rebalance flow.
  • + *
+ */ +public class ConsumerBackgroundThread extends KafkaThread { + private final Logger log; + private static final String CONSUMER_BACKGROUND_THREAD_PREFIX = "consumer_background_thread"; + private long retryBackoffMs; + private BackgroundState state = BackgroundState.DOWN; + private final BlockingQueue applicationEventQueue; + private final BlockingQueue backgroundEventQueue; + private final Time time; + + private final ConsumerConfig config; + + // control variables + private boolean running = false; + private Optional inflightEvent; + private boolean needCoordinator; + + public ConsumerBackgroundThread(ConsumerConfig config, + LogContext logContext, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue) { + this(Time.SYSTEM, + config, + logContext, + applicationEventQueue, + backgroundEventQueue); + } + + public ConsumerBackgroundThread(Time time, + ConsumerConfig config, + LogContext logContext, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue) { + super(CONSUMER_BACKGROUND_THREAD_PREFIX, true); + this.time = Time.SYSTEM; + this.log = logContext.logger(ConsumerBackgroundThread.class); + this.applicationEventQueue = applicationEventQueue; + this.backgroundEventQueue = backgroundEventQueue; + this.config = config; + setConfig(config); + this.state = BackgroundState.INITIALIZED; + } + + private void setConfig(ConsumerConfig config) { + this.retryBackoffMs = this.config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG); + } + + @Override + public void run() { + try { + log.debug("Background thread started"); + while(running) { + if (!inflightEvent.isPresent() && !applicationEventQueue.isEmpty()) { + inflightEvent = Optional.ofNullable(applicationEventQueue.poll()); + } + if(inflightEvent.isPresent()) { + needCoordinator = inflightEvent.get().needCoordinator; + tryConsumeInflightEvent(); + } + runStateMachine(); + this.wait(retryBackoffMs); + } + } catch(Exception e) { + // TODO: We need more fine grain exception and handle them differently + } finally { + log.debug("Background thread closed"); + } + } + + public void close() throws Exception { + + this.running = false; + transitionTo(BackgroundState.DOWN); + } + + private void runStateMachine() { + switch (state) { + case DOWN: + // closing + this.running = false; + return; + case INITIALIZED: + maybeFindCoordinator(); + break; + case FINDING_COORDINATOR: + maybeTransitionToStable(); + break; + case STABLE: + // poll coordinator + break; + } + } + + /** + * BackgroundState represents BackgroundThread's connection status to the coordinator. It can be in one of the four + * possible states. The state transition can be represented here: + *
+     *                 +--------------+
+     *         +-----> |     DOWN     |
+     *         |       +-----+--------+
+     *         |              |
+     *         |              v
+     *         |       +----+--+------+
+     *         |<------| INITIALIZED  | <----+
+     *         |       +-----+-+------+      |
+     *         |             | ^             |
+     *         |             v |             |
+     *         |       +--------------+      |
+     *         |<------| FINDING_COOD |      |
+     *         |       +------+-------+      |
+     *         |              |              |
+     *         |              v              |
+     *         |       +------+-------+      |
+     *(closed) +------ |   STABLE     | -----+ (disconnected)
+     *                 +------+-------+
+     * 
+     */
+    enum BackgroundState {
+        DOWN(1),
+        INITIALIZED(0, 2),
+        FINDING_COORDINATOR(1, 2, 3),
+        STABLE(0, 1, 2);
+
+        private final Set validTransition = new HashSet<>();
+        BackgroundState(final Integer... validTransitions) {
+            this.validTransition.addAll(Arrays.asList(validTransitions));
+        }
+        boolean isValidTransition(final BackgroundState newState) {
+            return validTransition.contains(newState.ordinal());
+        }
+    }
+
+    private void maybeTransitionToStable() {
+        // TODO: to be implemented
+    }
+    private void maybeFindCoordinator() {
+        if (!needCoordinator) {
+            return;
+        }
+        // find coordinator
+        transitionTo(BackgroundState.FINDING_COORDINATOR);
+    }
+
+    private void transitionTo(BackgroundState newState) {
+        if (!state.isValidTransition(newState)) {
+            throw new IllegalStateException("unable to transition from " + state + " to " + newState);
+        }
+        state = newState;
+    }
+
+    public void tryConsumeInflightEvent() {
+        // TODO: to be implemented
+    }
+}
\ No newline at end of file
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java
index 0b2e5a901d5d2..ec0e177e15c1b 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java
@@ -20,4 +20,16 @@
  * This is the abstract definition of the events created by the KafkaConsumer API
  */
 abstract public class ApplicationEvent {
+    public final EventTypes type;
+    public final boolean needCoordinator;
+
+    public ApplicationEvent(EventTypes type, boolean needCoordinator) {
+        this.type = type;
+        this.needCoordinator = needCoordinator;
+    }
+    public enum EventTypes {
+        COMMIT,
+        FETCH,
+        NOOP,
+    }
 }

From 684a0a11f90f6ebac72af7b013a478a822d178ca Mon Sep 17 00:00:00 2001
From: Philip Nee 
Date: Tue, 27 Sep 2022 16:06:00 -0700
Subject: [PATCH 19/45] Testing, background thread impl.

initialize backgorund thread.

refactor stuff a bit

Added unit tests

more unit tests

renaming and documentation

clean up

clean up

clean up
---
 .../AbstractPrototypeAsyncConsumer.java       |   3 +-
 .../internals/ApplicationEventProcessor.java  | 193 +++++++++++++++++
 .../internals/ConsumerBackgroundThread.java   | 196 ------------------
 .../internals/DefaultEventHandler.java        |  54 ++++-
 .../consumer/internals/EventProcessor.java    |  25 +++
 .../internals/events/ApplicationEvent.java    |   7 +-
 .../internals/events/BackgroundEvent.java     |   8 +
 .../internals/DefaultEventHandlerTest.java    | 121 +++++++++++
 8 files changed, 394 insertions(+), 213 deletions(-)
 create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/ApplicationEventProcessor.java
 delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java
 create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventProcessor.java
 create mode 100644 clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java

diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java
index f32fe4d74edf6..9a95168a5c19c 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java
@@ -26,7 +26,6 @@
 import java.time.Duration;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
@@ -114,7 +113,7 @@ public void commitSync(Duration timeout) {
      */
     private class CommitApplicationEvent extends ApplicationEvent {
         public CommitApplicationEvent() {
-            super(EventTypes.COMMIT, false);
+            super(EventType.COMMIT, false);
         }
         // this is the stubbed commitAsyncEvents
         CompletableFuture commitFuture = new CompletableFuture<>();
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ApplicationEventProcessor.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ApplicationEventProcessor.java
new file mode 100644
index 0000000000000..eec6d7728faea
--- /dev/null
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ApplicationEventProcessor.java
@@ -0,0 +1,193 @@
+/*
+ * 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 org.apache.kafka.clients.consumer.internals;
+
+import org.apache.kafka.clients.consumer.ConsumerConfig;
+import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent;
+import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent;
+import org.apache.kafka.common.utils.LogContext;
+import org.apache.kafka.common.utils.Time;
+import org.slf4j.Logger;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.BlockingQueue;
+
+/**
+ * This processor consumes {@code ApplicationEvent} and produces {@code BackgroundEvent}. The processor uses a state
+ * machine to manage its connection to the coordinator.  Because the coordinator connection is on demand, the processor
+ * can stay in the INITIALIZED state and continue to function.  When the coordinator is needed, the processor will
+ * issue a FindCoordinatorRequest and transition the state to the FINDING_COORDINATIOR stage, and may eventually end
+ * up in either INITALIZED state if the request failed, or STABLE if the request succeed.  It is also possible to
+ * transition to DOWN from any STATE.
+ * 
+ *                 +--------------+
+ *         +-----> |     DOWN     |
+ *         |       +-----+--------+
+ *         |              |
+ *         |              v
+ *         |       +----+--+------+
+ *         |<------| INITIALIZED  | <----+
+ *         |       +-----+-+------+      |
+ *         |             | ^             |
+ *         |             v |             |
+ *         |       +--------------+      |
+ *         |<------|  FIND_COORD  |      |
+ *         |       +------+-------+      |
+ *         |              |              |
+ *         |              v              |
+ *         |       +------+-------+      |
+ *(closed) +------ |   STABLE     | -----+ (disconnected)
+ *                 +------+-------+
+ * 
+ *
    + *
  • DOWN: The processor is closed or uninitialized.
  • + *
  • INITIALIZED: The processor has been initialized. It can start to consume {@code ApplicationEvent}.
  • + *
  • FINDING_COORDINATOR: The processor send out a {@code FindCoordinatorRequest} and is waiting for a + * response.
  • + *
  • STABLE: The background thread is connected to the coordinator. Note that rebalancing can only happen + * in this state.
  • + *
+ */ +public class ApplicationEventProcessor implements EventProcessor { + private final Logger log; + private final BlockingQueue applicationEventQueue; + private final BlockingQueue backgroundEventQueue; + private final Time time; + private final ConsumerConfig config; + + private long retryBackoffMs; + private BackgroundState state; + private boolean running = false; + private Optional inflightEvent; + + public ApplicationEventProcessor(ConsumerConfig config, + LogContext logContext, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue) { + this(Time.SYSTEM, + config, + logContext, + applicationEventQueue, + backgroundEventQueue); + } + + public ApplicationEventProcessor(Time time, + ConsumerConfig config, + LogContext logContext, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue) { + this.time = time; + this.log = logContext.logger(ApplicationEventProcessor.class); + this.applicationEventQueue = applicationEventQueue; + this.backgroundEventQueue = backgroundEventQueue; + this.config = config; + setConfig(); + this.state = BackgroundState.INITIALIZED; + this.inflightEvent = Optional.empty(); + } + + private void setConfig() { + this.retryBackoffMs = this.config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG); + } + + /** + * The main processor loop. + */ + @Override + public void run() { + try { + log.debug("ApplicationEventProcessor started"); + while (running) { + pollOnce(); + this.wait(retryBackoffMs); + } + } catch (Exception e) { + // TODO: Define fine grain exceptions here + } finally { + log.debug("ApplicationEventProcessor closed"); + } + } + + /** + * Process event from a single poll + */ + void pollOnce() { + inflightEvent = maybePollEvent(); + maybeConsumeInflightEvent(); + } + + public Optional maybePollEvent() { + if (inflightEvent.isPresent()) { + return Optional.empty(); + } + return Optional.ofNullable(applicationEventQueue.poll()); + } + + private void transitionTo(BackgroundState newState) { + if (!state.isValidTransition(newState)) { + throw new IllegalStateException("unable to transition from " + state + " to " + newState); + } + state = newState; + } + + public void maybeConsumeInflightEvent() { + if (!inflightEvent.isPresent()) { + return; + } + ApplicationEvent event = inflightEvent.get(); + if (!tryConsumeEvent(event)) { + return; + } + // clear inflight event upon successful consumption + inflightEvent = Optional.empty(); + } + + public boolean tryConsumeEvent(ApplicationEvent event) { + // consumption maybe return false when + // 1. need a coordinator and it's not available + // 2. other errors + return true; + } + + @Override + public void close() { + this.running = false; + transitionTo(BackgroundState.DOWN); + } + + /** + * Four states that the background thread can be in. + */ + enum BackgroundState { + DOWN(1), + INITIALIZED(0, 2), + FINDING_COORDINATOR(1, 2, 3), + STABLE(0, 1, 2); + + private final Set validTransition = new HashSet<>(); + BackgroundState(final Integer... validTransitions) { + this.validTransition.addAll(Arrays.asList(validTransitions)); + } + boolean isValidTransition(final BackgroundState newState) { + return validTransition.contains(newState.ordinal()); + } + } +} \ No newline at end of file diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java deleted file mode 100644 index 1ad53b6a74da6..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerBackgroundThread.java +++ /dev/null @@ -1,196 +0,0 @@ -/* - * 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 org.apache.kafka.clients.consumer.internals; - -import org.apache.kafka.clients.consumer.ConsumerConfig; -import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; -import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; -import org.apache.kafka.common.utils.KafkaThread; -import org.apache.kafka.common.utils.LogContext; -import org.apache.kafka.common.utils.Time; -import org.slf4j.Logger; - -import java.util.Arrays; -import java.util.HashSet; -import java.util.Optional; -import java.util.Set; -import java.util.concurrent.BlockingQueue; - -/** - * The background thread runs in the background and consumes the {@code ApplicationEvent} from the {@link org.apache.kafka.clients.consumer.KafkaConsumer} APIs. This class uses an event loop to drive the following important tasks: - *
    - *
  • Consuming and executing the {@code ApplicationEvent}.
  • - *
  • Maintaining the connection to the coordinator.
  • - *
  • Sending heartbeat.
  • - *
  • Autocommitting.
  • - *
  • Executing the rebalance flow.
  • - *
- */ -public class ConsumerBackgroundThread extends KafkaThread { - private final Logger log; - private static final String CONSUMER_BACKGROUND_THREAD_PREFIX = "consumer_background_thread"; - private long retryBackoffMs; - private BackgroundState state = BackgroundState.DOWN; - private final BlockingQueue applicationEventQueue; - private final BlockingQueue backgroundEventQueue; - private final Time time; - - private final ConsumerConfig config; - - // control variables - private boolean running = false; - private Optional inflightEvent; - private boolean needCoordinator; - - public ConsumerBackgroundThread(ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue) { - this(Time.SYSTEM, - config, - logContext, - applicationEventQueue, - backgroundEventQueue); - } - - public ConsumerBackgroundThread(Time time, - ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue) { - super(CONSUMER_BACKGROUND_THREAD_PREFIX, true); - this.time = Time.SYSTEM; - this.log = logContext.logger(ConsumerBackgroundThread.class); - this.applicationEventQueue = applicationEventQueue; - this.backgroundEventQueue = backgroundEventQueue; - this.config = config; - setConfig(config); - this.state = BackgroundState.INITIALIZED; - } - - private void setConfig(ConsumerConfig config) { - this.retryBackoffMs = this.config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG); - } - - @Override - public void run() { - try { - log.debug("Background thread started"); - while(running) { - if (!inflightEvent.isPresent() && !applicationEventQueue.isEmpty()) { - inflightEvent = Optional.ofNullable(applicationEventQueue.poll()); - } - if(inflightEvent.isPresent()) { - needCoordinator = inflightEvent.get().needCoordinator; - tryConsumeInflightEvent(); - } - runStateMachine(); - this.wait(retryBackoffMs); - } - } catch(Exception e) { - // TODO: We need more fine grain exception and handle them differently - } finally { - log.debug("Background thread closed"); - } - } - - public void close() throws Exception { - - this.running = false; - transitionTo(BackgroundState.DOWN); - } - - private void runStateMachine() { - switch (state) { - case DOWN: - // closing - this.running = false; - return; - case INITIALIZED: - maybeFindCoordinator(); - break; - case FINDING_COORDINATOR: - maybeTransitionToStable(); - break; - case STABLE: - // poll coordinator - break; - } - } - - /** - * BackgroundState represents BackgroundThread's connection status to the coordinator. It can be in one of the four - * possible states. The state transition can be represented here: - *
-     *                 +--------------+
-     *         +-----> |     DOWN     |
-     *         |       +-----+--------+
-     *         |              |
-     *         |              v
-     *         |       +----+--+------+
-     *         |<------| INITIALIZED  | <----+
-     *         |       +-----+-+------+      |
-     *         |             | ^             |
-     *         |             v |             |
-     *         |       +--------------+      |
-     *         |<------| FINDING_COOD |      |
-     *         |       +------+-------+      |
-     *         |              |              |
-     *         |              v              |
-     *         |       +------+-------+      |
-     *(closed) +------ |   STABLE     | -----+ (disconnected)
-     *                 +------+-------+
-     * 
-     */
-    enum BackgroundState {
-        DOWN(1),
-        INITIALIZED(0, 2),
-        FINDING_COORDINATOR(1, 2, 3),
-        STABLE(0, 1, 2);
-
-        private final Set validTransition = new HashSet<>();
-        BackgroundState(final Integer... validTransitions) {
-            this.validTransition.addAll(Arrays.asList(validTransitions));
-        }
-        boolean isValidTransition(final BackgroundState newState) {
-            return validTransition.contains(newState.ordinal());
-        }
-    }
-
-    private void maybeTransitionToStable() {
-        // TODO: to be implemented
-    }
-    private void maybeFindCoordinator() {
-        if (!needCoordinator) {
-            return;
-        }
-        // find coordinator
-        transitionTo(BackgroundState.FINDING_COORDINATOR);
-    }
-
-    private void transitionTo(BackgroundState newState) {
-        if (!state.isValidTransition(newState)) {
-            throw new IllegalStateException("unable to transition from " + state + " to " + newState);
-        }
-        state = newState;
-    }
-
-    public void tryConsumeInflightEvent() {
-        // TODO: to be implemented
-    }
-}
\ No newline at end of file
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
index ec1368ed801f8..36eba0d8fc247 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
@@ -16,45 +16,77 @@
  */
 package org.apache.kafka.clients.consumer.internals;
 
+import org.apache.kafka.clients.consumer.ConsumerConfig;
 import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent;
 import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent;
 import org.apache.kafka.clients.consumer.internals.events.EventHandler;
+import org.apache.kafka.common.utils.KafkaThread;
+import org.apache.kafka.common.utils.LogContext;
 
 import java.util.Optional;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 
 /**
- * This class interfaces the KafkaConsumer and the background thread.  It allows the caller to enqueue {@link ApplicationEvent}
- * to be consumed by the background thread and poll {@linkBackgroundEvent} produced by the background thread.
+ * An {@code EventHandler} that uses a single background thread to consume {@code ApplicationEvent} and produce
+ * {@code BackgroundEvent} from the {@ConsumerBackgroundThread}.
  */
 public class DefaultEventHandler implements EventHandler {
-    private final BlockingQueue applicationEvents;
-    private final BlockingQueue backgroundEvents;
+    private final BlockingQueue applicationEventQueue;
+    private final BlockingQueue backgroundEventQueue;
+    private final EventProcessor eventProcessor;
+    private final KafkaThread backgroundThread;
 
-    public DefaultEventHandler() {
-        this.applicationEvents = new LinkedBlockingQueue<>();
-        this.backgroundEvents = new LinkedBlockingQueue<>();
-        // TODO: a concreted implementation of how requests are being consumed, and how responses are being produced.
+    public DefaultEventHandler(ConsumerConfig config, LogContext logcontext) {
+        this.applicationEventQueue = new LinkedBlockingQueue<>();
+        this.backgroundEventQueue = new LinkedBlockingQueue<>();
+        this.eventProcessor = new ApplicationEventProcessor(
+                config,
+                logcontext,
+                applicationEventQueue,
+                backgroundEventQueue);
+        this.backgroundThread = new KafkaThread("consumer_background_thread", eventProcessor, true);
+        backgroundThread.start();
+
+    }
+
+    // VisibleForTesting
+    DefaultEventHandler(EventProcessor runnable,
+                         BlockingQueue applicationEventQueue,
+                         BlockingQueue backgroundEventQueue) {
+        this.eventProcessor = runnable;
+        this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true);
+        this.applicationEventQueue = applicationEventQueue;
+        this.backgroundEventQueue = backgroundEventQueue;
+        backgroundThread.start();
     }
 
     @Override
     public Optional poll() {
-        return Optional.ofNullable(backgroundEvents.poll());
+        return Optional.ofNullable(backgroundEventQueue.poll());
     }
 
     @Override
     public boolean isEmpty() {
-        return backgroundEvents.isEmpty();
+        return backgroundEventQueue.isEmpty();
     }
 
     @Override
     public boolean add(ApplicationEvent event) {
         try {
-            return applicationEvents.add(event);
+            return applicationEventQueue.add(event);
         } catch (IllegalStateException e) {
             // swallow the capacity restriction exception
             return false;
         }
     }
+
+    public void close() {
+        try {
+            this.eventProcessor.close();
+            // close logic
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
 }
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventProcessor.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventProcessor.java
new file mode 100644
index 0000000000000..c4ee9501c1c0c
--- /dev/null
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventProcessor.java
@@ -0,0 +1,25 @@
+/*
+ * 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 org.apache.kafka.clients.consumer.internals;
+
+import java.io.Closeable;
+
+/**
+ * This interfaces the DefaultEventHandler and the underlying processing thread.
+ */
+public interface EventProcessor extends Runnable, Closeable {
+}
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java
index ec0e177e15c1b..15d7798f9624e 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java
@@ -20,16 +20,15 @@
  * This is the abstract definition of the events created by the KafkaConsumer API
  */
 abstract public class ApplicationEvent {
-    public final EventTypes type;
+    public final EventType type;
     public final boolean needCoordinator;
 
-    public ApplicationEvent(EventTypes type, boolean needCoordinator) {
+    public ApplicationEvent(EventType type, boolean needCoordinator) {
         this.type = type;
         this.needCoordinator = needCoordinator;
     }
-    public enum EventTypes {
+    public enum EventType {
         COMMIT,
-        FETCH,
         NOOP,
     }
 }
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/BackgroundEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/BackgroundEvent.java
index 8870e179d874e..89eac1048d95a 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/BackgroundEvent.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/BackgroundEvent.java
@@ -20,4 +20,12 @@
  * This is the abstract definition of the events created by the background thread.
  */
 abstract public class BackgroundEvent {
+    public final EventType type;
+
+    public BackgroundEvent(EventType type) {
+        this.type = type;
+    }
+    public enum EventType {
+        NOOP,
+    }
 }
diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java
new file mode 100644
index 0000000000000..209d39c9cf311
--- /dev/null
+++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java
@@ -0,0 +1,121 @@
+/*
+ * 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 org.apache.kafka.clients.consumer.internals;
+
+import org.apache.kafka.clients.consumer.ConsumerConfig;
+import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent;
+import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent;
+import org.apache.kafka.clients.consumer.internals.events.EventHandler;
+import org.apache.kafka.common.serialization.StringDeserializer;
+import org.apache.kafka.common.utils.LogContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.Optional;
+import java.util.Properties;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG;
+import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class DefaultEventHandlerTest {
+    private final Properties properties = new Properties();
+
+    @BeforeEach
+    public void setup() {
+        properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class);
+        properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class);
+    }
+
+    @Test
+    public void testPollAndAdd() {
+        EventHandler handler = new DefaultEventHandler(
+                new ConsumerConfig(properties),
+                new LogContext());
+        assertTrue(handler.isEmpty());
+        assertTrue(!handler.poll().isPresent());
+        handler.add(new StubbedApplicationEvent());
+        assertTrue(handler.isEmpty());
+    }
+
+    @Test
+    public void testRunOnceBackgroundThread() {
+        BlockingQueue applicationEventQueue = new LinkedBlockingQueue<>();
+        BlockingQueue backgroundEventQueue = new LinkedBlockingQueue<>();
+        EventHandler eventHandler = new DefaultEventHandler(
+                new RunOnceBackgroundThread(applicationEventQueue, backgroundEventQueue),
+                applicationEventQueue,
+                backgroundEventQueue);
+        assertTrue(eventHandler.add(new StubbedApplicationEvent("hello-world")));
+        assertFalse(eventHandler.isEmpty());
+        Optional event = eventHandler.poll();
+        assertTrue(event.isPresent());
+        assertTrue(event.get() instanceof RunOnceBackgroundThread.NoopBackgroundEvent);
+        assertEquals(BackgroundEvent.EventType.NOOP, event.get().type);
+        assertEquals("hello-world", ((RunOnceBackgroundThread.NoopBackgroundEvent) event.get()).message);
+    }
+
+    private class StubbedApplicationEvent extends ApplicationEvent {
+        public final String message;
+        public StubbedApplicationEvent() {
+            this("");
+        }
+
+        public StubbedApplicationEvent(String message) {
+            super(EventType.NOOP, false);
+            this.message = message;
+        }
+    }
+
+    private class RunOnceBackgroundThread implements EventProcessor {
+        private BlockingQueue applicationEventQueue;
+        private BlockingQueue backgroundEventQueue;
+        public RunOnceBackgroundThread(BlockingQueue applicationEvents,
+                                       BlockingQueue backgroundEventQueue) {
+            this.applicationEventQueue = applicationEvents;
+            this.backgroundEventQueue = backgroundEventQueue;
+        }
+        @Override
+        public void run() {
+            while (applicationEventQueue.isEmpty()) { }
+            ApplicationEvent event = applicationEventQueue.poll();
+            String message = ((StubbedApplicationEvent) event).message;
+            backgroundEventQueue.add(new NoopBackgroundEvent(message));
+        }
+
+        @Override
+        public void close() {
+        }
+
+        class NoopBackgroundEvent extends BackgroundEvent {
+            public String message;
+            public NoopBackgroundEvent(String message) {
+                super(EventType.NOOP);
+                this.message = message;
+            }
+
+            @Override
+            public String toString() {
+                return type + ":" + message;
+            }
+        }
+    }
+}

From c80059a9180c13f61b67e8d077992edeb2b7fc49 Mon Sep 17 00:00:00 2001
From: Philip Nee 
Date: Fri, 30 Sep 2022 13:23:15 -0700
Subject: [PATCH 20/45] renaming for clarity

---
 .../internals/DefaultEventHandler.java        |  2 +-
 ...cessor.java => DefaultEventProcessor.java} | 26 +++++++++----------
 2 files changed, 14 insertions(+), 14 deletions(-)
 rename clients/src/main/java/org/apache/kafka/clients/consumer/internals/{ApplicationEventProcessor.java => DefaultEventProcessor.java} (87%)

diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
index 36eba0d8fc247..53fede0397769 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
@@ -40,7 +40,7 @@ public class DefaultEventHandler implements EventHandler {
     public DefaultEventHandler(ConsumerConfig config, LogContext logcontext) {
         this.applicationEventQueue = new LinkedBlockingQueue<>();
         this.backgroundEventQueue = new LinkedBlockingQueue<>();
-        this.eventProcessor = new ApplicationEventProcessor(
+        this.eventProcessor = new DefaultEventProcessor(
                 config,
                 logcontext,
                 applicationEventQueue,
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ApplicationEventProcessor.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
similarity index 87%
rename from clients/src/main/java/org/apache/kafka/clients/consumer/internals/ApplicationEventProcessor.java
rename to clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
index eec6d7728faea..ada2933cfca21 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ApplicationEventProcessor.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
@@ -66,7 +66,7 @@
  * in this state.
  * 
  */
-public class ApplicationEventProcessor implements EventProcessor {
+public class DefaultEventProcessor implements EventProcessor {
     private final Logger log;
     private final BlockingQueue applicationEventQueue;
     private final BlockingQueue backgroundEventQueue;
@@ -78,10 +78,10 @@ public class ApplicationEventProcessor implements EventProcessor {
     private boolean running = false;
     private Optional inflightEvent;
 
-    public ApplicationEventProcessor(ConsumerConfig config,
-                                     LogContext logContext,
-                                     BlockingQueue applicationEventQueue,
-                                     BlockingQueue backgroundEventQueue) {
+    public DefaultEventProcessor(ConsumerConfig config,
+                                 LogContext logContext,
+                                 BlockingQueue applicationEventQueue,
+                                 BlockingQueue backgroundEventQueue) {
         this(Time.SYSTEM,
                 config,
                 logContext,
@@ -89,13 +89,13 @@ public ApplicationEventProcessor(ConsumerConfig config,
                 backgroundEventQueue);
     }
 
-    public ApplicationEventProcessor(Time time,
-                                     ConsumerConfig config,
-                                     LogContext logContext,
-                                     BlockingQueue applicationEventQueue,
-                                     BlockingQueue backgroundEventQueue) {
+    public DefaultEventProcessor(Time time,
+                                 ConsumerConfig config,
+                                 LogContext logContext,
+                                 BlockingQueue applicationEventQueue,
+                                 BlockingQueue backgroundEventQueue) {
         this.time = time;
-        this.log = logContext.logger(ApplicationEventProcessor.class);
+        this.log = logContext.logger(DefaultEventProcessor.class);
         this.applicationEventQueue = applicationEventQueue;
         this.backgroundEventQueue = backgroundEventQueue;
         this.config = config;
@@ -114,7 +114,7 @@ private void setConfig() {
     @Override
     public void run() {
         try {
-            log.debug("ApplicationEventProcessor started");
+            log.debug("DefaultEventProcessor started");
             while (running) {
                 pollOnce();
                 this.wait(retryBackoffMs);
@@ -122,7 +122,7 @@ public void run() {
         } catch (Exception e) {
             // TODO: Define fine grain exceptions here
         } finally {
-            log.debug("ApplicationEventProcessor closed");
+            log.debug("DefaultEventProcessor closed");
         }
     }
 

From 480eb9ab80175d71fe4b68efe7d65025477118c0 Mon Sep 17 00:00:00 2001
From: Philip Nee 
Date: Fri, 30 Sep 2022 13:38:38 -0700
Subject: [PATCH 21/45] fixes

---
 .../internals/DefaultEventProcessor.java      | 31 +++++++------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
index ada2933cfca21..5a99c8776c583 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
@@ -130,37 +130,28 @@ public void run() {
      * Process event from a single poll
      */
     void pollOnce() {
-        inflightEvent = maybePollEvent();
-        maybeConsumeInflightEvent();
+        this.inflightEvent = maybePollEvent();
+        if (this.inflightEvent.isPresent() && maybeConsumeInflightEvent(this.inflightEvent.get())) {
+            // clear inflight event upon successful consumption
+            this.inflightEvent = Optional.empty();
+        }
     }
 
     public Optional maybePollEvent() {
-        if (inflightEvent.isPresent()) {
-            return Optional.empty();
+        if (this.inflightEvent.isPresent()) {
+            return this.inflightEvent;
         }
-        return Optional.ofNullable(applicationEventQueue.poll());
+        return Optional.ofNullable(this.applicationEventQueue.poll());
     }
 
     private void transitionTo(BackgroundState newState) {
         if (!state.isValidTransition(newState)) {
-            throw new IllegalStateException("unable to transition from " + state + " to " + newState);
-        }
-        state = newState;
-    }
-
-    public void maybeConsumeInflightEvent() {
-        if (!inflightEvent.isPresent()) {
-            return;
-        }
-        ApplicationEvent event = inflightEvent.get();
-        if (!tryConsumeEvent(event)) {
-            return;
+            throw new IllegalStateException("unable to transition from " + this.state + " to " + newState);
         }
-        // clear inflight event upon successful consumption
-        inflightEvent = Optional.empty();
+        this.state = newState;
     }
 
-    public boolean tryConsumeEvent(ApplicationEvent event) {
+    public boolean maybeConsumeInflightEvent(ApplicationEvent event) {
         // consumption maybe return false when
         // 1. need a coordinator and it's not available
         // 2. other errors

From 422eed49f010be66c9118ab84adbc2ff1b664bf8 Mon Sep 17 00:00:00 2001
From: Philip Nee 
Date: Sat, 1 Oct 2022 14:04:39 -0700
Subject: [PATCH 22/45] Refactor the background to make clear it is a network
 io thread

network IO thread
---
 ...sor.java => BackgroundThreadRunnable.java} |   5 +-
 .../DefaultBackgroundThreadRunnable.java      | 239 ++++++++++++++++++
 .../internals/DefaultEventHandler.java        |  57 ++++-
 .../internals/DefaultEventProcessor.java      | 184 --------------
 .../internals/DefaultEventHandlerTest.java    |  60 +++--
 5 files changed, 334 insertions(+), 211 deletions(-)
 rename clients/src/main/java/org/apache/kafka/clients/consumer/internals/{EventProcessor.java => BackgroundThreadRunnable.java} (81%)
 create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java
 delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java

diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventProcessor.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java
similarity index 81%
rename from clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventProcessor.java
rename to clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java
index c4ee9501c1c0c..a327055aebcd6 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventProcessor.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java
@@ -19,7 +19,8 @@
 import java.io.Closeable;
 
 /**
- * This interfaces the DefaultEventHandler and the underlying processing thread.
+ * The {@code EventHandler} constructs a thread that runs {@code BackgroundThreadRunnable} to handle network requests
+ * and responses.
  */
-public interface EventProcessor extends Runnable, Closeable {
+public interface BackgroundThreadRunnable extends Runnable, Closeable {
 }
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java
new file mode 100644
index 0000000000000..cecdd79b5eb8e
--- /dev/null
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java
@@ -0,0 +1,239 @@
+/*
+ * 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 org.apache.kafka.clients.consumer.internals;
+
+import org.apache.kafka.clients.ApiVersions;
+import org.apache.kafka.clients.ClientUtils;
+import org.apache.kafka.clients.CommonClientConfigs;
+import org.apache.kafka.clients.NetworkClient;
+import org.apache.kafka.clients.consumer.ConsumerConfig;
+import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent;
+import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent;
+import org.apache.kafka.common.KafkaException;
+import org.apache.kafka.common.internals.ClusterResourceListeners;
+import org.apache.kafka.common.metrics.Metrics;
+import org.apache.kafka.common.metrics.Sensor;
+import org.apache.kafka.common.network.ChannelBuilder;
+import org.apache.kafka.common.network.Selector;
+import org.apache.kafka.common.utils.LogContext;
+import org.apache.kafka.common.utils.Time;
+import org.apache.kafka.common.utils.Utils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.InetSocketAddress;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+
+/**
+ * Lives inside of the {@code DefaultEventHandler}, and consumes {@code ApplicationEvent} and produces
+ * {@code BackgroundEvent}.
+ */
+public class DefaultBackgroundThreadRunnable implements BackgroundThreadRunnable {
+    private static final String CLIENT_ID_METRIC_TAG = "client-id";
+    private static final String METRIC_GRP_PREFIX = "consumer";
+
+    private final Time time;
+    private final Logger log;
+    private final BlockingQueue applicationEventQueue;
+    private final BlockingQueue backgroundEventQueue;
+    private final ConsumerNetworkClient networkClient;
+
+    private final ConsumerConfig config;
+    private final Metrics metrics;
+    private final SubscriptionState subscriptions;
+    private final ConsumerMetadata metadata;
+    private String clientId;
+    private long retryBackoffMs;
+    private int heartbeatIntervalMs;
+    private boolean running = false;
+    private Optional inflightEvent;
+
+    public DefaultBackgroundThreadRunnable(ConsumerConfig config,
+                                           LogContext logContext,
+                                           BlockingQueue applicationEventQueue,
+                                           BlockingQueue backgroundEventQueue,
+                                           SubscriptionState subscriptions,
+                                           ApiVersions apiVersions,
+                                           Metrics metrics,
+                                           ClusterResourceListeners clusterResourceListeners,
+                                           Sensor fetcherThrottleTimeSensor) {
+        this(Time.SYSTEM,
+                config,
+                logContext,
+                applicationEventQueue,
+                backgroundEventQueue,
+                subscriptions,
+                apiVersions,
+                metrics,
+                clusterResourceListeners,
+                fetcherThrottleTimeSensor);
+    }
+
+    public DefaultBackgroundThreadRunnable(Time time,
+                                           ConsumerConfig config,
+                                           LogContext logContext,
+                                           BlockingQueue applicationEventQueue,
+                                           BlockingQueue backgroundEventQueue,
+                                           SubscriptionState subscriptions,
+                                           ApiVersions apiVersions,
+                                           Metrics metrics,
+                                           ClusterResourceListeners clusterResourceListeners,
+                                           Sensor fetcherThrottleTimeSensor) {
+        try {
+            this.time = time;
+            this.log = logContext.logger(DefaultBackgroundThreadRunnable.class);
+            this.applicationEventQueue = applicationEventQueue;
+            this.backgroundEventQueue = backgroundEventQueue;
+            this.config = config;
+            setConfig();
+            this.inflightEvent = Optional.empty();
+            this.subscriptions = subscriptions;
+            this.metrics = metrics;
+            this.metadata = bootstrapMetadata(clusterResourceListeners, logContext);
+            ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext);
+            NetworkClient netClient = new NetworkClient(
+                    new Selector(config.getLong(ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), metrics, time, METRIC_GRP_PREFIX, channelBuilder, logContext),
+                    this.metadata,
+                    clientId,
+                    100, // a fixed large enough value will suffice for max in-flight requests
+                    config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG),
+                    config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MAX_MS_CONFIG),
+                    config.getInt(ConsumerConfig.SEND_BUFFER_CONFIG),
+                    config.getInt(ConsumerConfig.RECEIVE_BUFFER_CONFIG),
+                    config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG),
+                    config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_CONFIG),
+                    config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG),
+                    time,
+                    true,
+                    apiVersions,
+                    fetcherThrottleTimeSensor,
+                    logContext);
+            this.networkClient = new ConsumerNetworkClient(
+                    logContext,
+                    netClient,
+                    metadata,
+                    time,
+                    retryBackoffMs,
+                    config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG),
+                    heartbeatIntervalMs);
+        } catch (Exception e) {
+            // now propagate the exception
+            throw new KafkaException("Failed to construct background processor", e);
+        }
+    }
+
+    // VisibleForTesting
+    DefaultBackgroundThreadRunnable(Time time,
+                                    ConsumerConfig config,
+                                    BlockingQueue applicationEventQueue,
+                                    BlockingQueue backgroundEventQueue,
+                                    SubscriptionState subscriptions,
+                                    ConsumerMetadata metadata,
+                                    ConsumerNetworkClient client) {
+        this.time = time;
+        this.config = config;
+        this.log = LoggerFactory.getLogger(getClass());
+        this.applicationEventQueue = applicationEventQueue;
+        this.backgroundEventQueue = backgroundEventQueue;
+        this.subscriptions = subscriptions;
+        this.metadata = metadata;
+        this.networkClient = client;
+        this.metrics = new Metrics();
+    }
+
+    private void setConfig() {
+        this.retryBackoffMs = this.config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG);
+        this.clientId = config.getString(CommonClientConfigs.CLIENT_ID_CONFIG);
+        this.heartbeatIntervalMs = config.getInt(ConsumerConfig.HEARTBEAT_INTERVAL_MS_CONFIG);
+    }
+
+    /**
+     * The main processor loop.
+     */
+    @Override
+    public void run() {
+        try {
+            log.debug("DefaultBackgroundThreadRunnable started");
+            while (running) {
+                pollOnce();
+                this.wait(retryBackoffMs);
+            }
+        } catch (Exception e) {
+            // TODO: Define fine grain exceptions here
+        } finally {
+            log.debug("DefaultBackgroundThreadRunnable closed");
+        }
+    }
+
+    /**
+     * Process event from a single poll
+     */
+    void pollOnce() {
+        this.inflightEvent = maybePollEvent();
+        if (this.inflightEvent.isPresent() && maybeConsumeInflightEvent(this.inflightEvent.get())) {
+            // clear inflight event upon successful consumption
+            this.inflightEvent = Optional.empty();
+        }
+        networkClient.pollNoWakeup();
+    }
+
+    public Optional maybePollEvent() {
+        if (this.inflightEvent.isPresent()) {
+            return this.inflightEvent;
+        }
+        return Optional.ofNullable(this.applicationEventQueue.poll());
+    }
+
+    /**
+     * ApplicationEvent are consumed here.
+     * @param event an {@link ApplicationEvent}
+     * @return true when successfully consumed the event.
+     */
+    public boolean maybeConsumeInflightEvent(ApplicationEvent event) {
+        switch (event.type) {
+            case NOOP:
+                log.info("consuming a NOOP event");
+                return true;
+            default:
+                inflightEvent = Optional.empty();
+                log.info("unsupported event type: {}", event.type);
+        }
+        return false;
+    }
+
+    @Override
+    public void close() {
+        this.running = false;
+        Utils.closeQuietly(networkClient, "consumer network client");
+    }
+
+    private ConsumerMetadata bootstrapMetadata(ClusterResourceListeners clusterResourceListeners, LogContext logContext) {
+        ConsumerMetadata metadata = new ConsumerMetadata(retryBackoffMs,
+                config.getLong(ConsumerConfig.METADATA_MAX_AGE_CONFIG),
+                !config.getBoolean(ConsumerConfig.EXCLUDE_INTERNAL_TOPICS_CONFIG),
+                config.getBoolean(ConsumerConfig.ALLOW_AUTO_CREATE_TOPICS_CONFIG),
+                this.subscriptions,
+                logContext, clusterResourceListeners);
+        List addresses = ClientUtils.parseAndValidateAddresses(
+                config.getList(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG), config.getString(ConsumerConfig.CLIENT_DNS_LOOKUP_CONFIG));
+        metadata.bootstrap(addresses);
+        return metadata;
+    }
+}
\ No newline at end of file
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
index 53fede0397769..e66ec164c62e7 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java
@@ -16,12 +16,17 @@
  */
 package org.apache.kafka.clients.consumer.internals;
 
+import org.apache.kafka.clients.ApiVersions;
 import org.apache.kafka.clients.consumer.ConsumerConfig;
 import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent;
 import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent;
 import org.apache.kafka.clients.consumer.internals.events.EventHandler;
+import org.apache.kafka.common.internals.ClusterResourceListeners;
+import org.apache.kafka.common.metrics.Metrics;
+import org.apache.kafka.common.metrics.Sensor;
 import org.apache.kafka.common.utils.KafkaThread;
 import org.apache.kafka.common.utils.LogContext;
+import org.apache.kafka.common.utils.Time;
 
 import java.util.Optional;
 import java.util.concurrent.BlockingQueue;
@@ -34,27 +39,59 @@
 public class DefaultEventHandler implements EventHandler {
     private final BlockingQueue applicationEventQueue;
     private final BlockingQueue backgroundEventQueue;
-    private final EventProcessor eventProcessor;
+    private final BackgroundThreadRunnable runnable;
     private final KafkaThread backgroundThread;
 
-    public DefaultEventHandler(ConsumerConfig config, LogContext logcontext) {
+    public DefaultEventHandler(ConsumerConfig config, LogContext logcontext,
+                               SubscriptionState subscriptionState,
+                               Metrics metrics,
+                               ClusterResourceListeners clusterResourceListeners,
+                               Sensor fetcherThrottleTimeSensor,
+                               ApiVersions apiVersions) {
         this.applicationEventQueue = new LinkedBlockingQueue<>();
         this.backgroundEventQueue = new LinkedBlockingQueue<>();
-        this.eventProcessor = new DefaultEventProcessor(
+        this.runnable = new DefaultBackgroundThreadRunnable(
                 config,
                 logcontext,
                 applicationEventQueue,
-                backgroundEventQueue);
-        this.backgroundThread = new KafkaThread("consumer_background_thread", eventProcessor, true);
+                backgroundEventQueue,
+                subscriptionState,
+                apiVersions,
+                metrics,
+                clusterResourceListeners,
+                fetcherThrottleTimeSensor);
+        this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true);
         backgroundThread.start();
 
     }
 
     // VisibleForTesting
-    DefaultEventHandler(EventProcessor runnable,
-                         BlockingQueue applicationEventQueue,
-                         BlockingQueue backgroundEventQueue) {
-        this.eventProcessor = runnable;
+    DefaultEventHandler(BackgroundThreadRunnable runnable,
+                        BlockingQueue applicationEventQueue,
+                        BlockingQueue backgroundEventQueue) {
+        this.runnable = runnable;
+        this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true);
+        this.applicationEventQueue = applicationEventQueue;
+        this.backgroundEventQueue = backgroundEventQueue;
+        backgroundThread.start();
+    }
+
+    // VisibleForTesting
+    DefaultEventHandler(Time time,
+                        ConsumerConfig config,
+                        BlockingQueue applicationEventQueue,
+                        BlockingQueue backgroundEventQueue,
+                        SubscriptionState subscriptionState,
+                        ConsumerMetadata metadata,
+                        ConsumerNetworkClient networkClient) {
+        this.runnable = new DefaultBackgroundThreadRunnable(
+                time,
+                config,
+                applicationEventQueue,
+                backgroundEventQueue,
+                subscriptionState,
+                metadata,
+                networkClient);
         this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true);
         this.applicationEventQueue = applicationEventQueue;
         this.backgroundEventQueue = backgroundEventQueue;
@@ -83,7 +120,7 @@ public boolean add(ApplicationEvent event) {
 
     public void close() {
         try {
-            this.eventProcessor.close();
+            this.runnable.close();
             // close logic
         } catch (Exception e) {
             throw new RuntimeException(e);
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
deleted file mode 100644
index 5a99c8776c583..0000000000000
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventProcessor.java
+++ /dev/null
@@ -1,184 +0,0 @@
-/*
- * 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 org.apache.kafka.clients.consumer.internals;
-
-import org.apache.kafka.clients.consumer.ConsumerConfig;
-import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent;
-import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent;
-import org.apache.kafka.common.utils.LogContext;
-import org.apache.kafka.common.utils.Time;
-import org.slf4j.Logger;
-
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Optional;
-import java.util.Set;
-import java.util.concurrent.BlockingQueue;
-
-/**
- * This processor consumes {@code ApplicationEvent} and produces {@code BackgroundEvent}. The processor uses a state
- * machine to manage its connection to the coordinator.  Because the coordinator connection is on demand, the processor
- * can stay in the INITIALIZED state and continue to function.  When the coordinator is needed, the processor will
- * issue a FindCoordinatorRequest and transition the state to the FINDING_COORDINATIOR stage, and may eventually end
- * up in either INITALIZED state if the request failed, or STABLE if the request succeed.  It is also possible to
- * transition to DOWN from any STATE.
- * 
- *                 +--------------+
- *         +-----> |     DOWN     |
- *         |       +-----+--------+
- *         |              |
- *         |              v
- *         |       +----+--+------+
- *         |<------| INITIALIZED  | <----+
- *         |       +-----+-+------+      |
- *         |             | ^             |
- *         |             v |             |
- *         |       +--------------+      |
- *         |<------|  FIND_COORD  |      |
- *         |       +------+-------+      |
- *         |              |              |
- *         |              v              |
- *         |       +------+-------+      |
- *(closed) +------ |   STABLE     | -----+ (disconnected)
- *                 +------+-------+
- * 
- *
    - *
  • DOWN: The processor is closed or uninitialized.
  • - *
  • INITIALIZED: The processor has been initialized. It can start to consume {@code ApplicationEvent}.
  • - *
  • FINDING_COORDINATOR: The processor send out a {@code FindCoordinatorRequest} and is waiting for a - * response.
  • - *
  • STABLE: The background thread is connected to the coordinator. Note that rebalancing can only happen - * in this state.
  • - *
- */ -public class DefaultEventProcessor implements EventProcessor { - private final Logger log; - private final BlockingQueue applicationEventQueue; - private final BlockingQueue backgroundEventQueue; - private final Time time; - private final ConsumerConfig config; - - private long retryBackoffMs; - private BackgroundState state; - private boolean running = false; - private Optional inflightEvent; - - public DefaultEventProcessor(ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue) { - this(Time.SYSTEM, - config, - logContext, - applicationEventQueue, - backgroundEventQueue); - } - - public DefaultEventProcessor(Time time, - ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue) { - this.time = time; - this.log = logContext.logger(DefaultEventProcessor.class); - this.applicationEventQueue = applicationEventQueue; - this.backgroundEventQueue = backgroundEventQueue; - this.config = config; - setConfig(); - this.state = BackgroundState.INITIALIZED; - this.inflightEvent = Optional.empty(); - } - - private void setConfig() { - this.retryBackoffMs = this.config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG); - } - - /** - * The main processor loop. - */ - @Override - public void run() { - try { - log.debug("DefaultEventProcessor started"); - while (running) { - pollOnce(); - this.wait(retryBackoffMs); - } - } catch (Exception e) { - // TODO: Define fine grain exceptions here - } finally { - log.debug("DefaultEventProcessor closed"); - } - } - - /** - * Process event from a single poll - */ - void pollOnce() { - this.inflightEvent = maybePollEvent(); - if (this.inflightEvent.isPresent() && maybeConsumeInflightEvent(this.inflightEvent.get())) { - // clear inflight event upon successful consumption - this.inflightEvent = Optional.empty(); - } - } - - public Optional maybePollEvent() { - if (this.inflightEvent.isPresent()) { - return this.inflightEvent; - } - return Optional.ofNullable(this.applicationEventQueue.poll()); - } - - private void transitionTo(BackgroundState newState) { - if (!state.isValidTransition(newState)) { - throw new IllegalStateException("unable to transition from " + this.state + " to " + newState); - } - this.state = newState; - } - - public boolean maybeConsumeInflightEvent(ApplicationEvent event) { - // consumption maybe return false when - // 1. need a coordinator and it's not available - // 2. other errors - return true; - } - - @Override - public void close() { - this.running = false; - transitionTo(BackgroundState.DOWN); - } - - /** - * Four states that the background thread can be in. - */ - enum BackgroundState { - DOWN(1), - INITIALIZED(0, 2), - FINDING_COORDINATOR(1, 2, 3), - STABLE(0, 1, 2); - - private final Set validTransition = new HashSet<>(); - BackgroundState(final Integer... validTransitions) { - this.validTransition.addAll(Arrays.asList(validTransitions)); - } - boolean isValidTransition(final BackgroundState newState) { - return validTransition.contains(newState.ordinal()); - } - } -} \ No newline at end of file diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 209d39c9cf311..3e02fc20793d9 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -16,12 +16,17 @@ */ package org.apache.kafka.clients.consumer.internals; +import org.apache.kafka.clients.MockClient; import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.clients.consumer.OffsetResetStrategy; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.serialization.StringDeserializer; import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.MockTime; +import org.apache.kafka.common.utils.Time; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -38,22 +43,40 @@ public class DefaultEventHandlerTest { private final Properties properties = new Properties(); + private SubscriptionState subscriptions; + @BeforeEach public void setup() { + subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); } @Test - public void testPollAndAdd() { - EventHandler handler = new DefaultEventHandler( + public void testBasicPollAndAddWithNoopEvent() { + LogContext logContext = new LogContext(); + Time time = new MockTime(); + ConsumerMetadata metadata = newConsumerMetadata(false); + MockClient client = new MockClient(time, metadata); + ConsumerNetworkClient consumerClient = new ConsumerNetworkClient(logContext, client, metadata, time, + 100, 1000, 100); + DefaultEventHandler handler = new DefaultEventHandler( + time, new ConsumerConfig(properties), - new LogContext()); + new LinkedBlockingQueue<>(), + new LinkedBlockingQueue<>(), + subscriptions, + metadata, + consumerClient); + assertTrue(client.active()); assertTrue(handler.isEmpty()); assertTrue(!handler.poll().isPresent()); - handler.add(new StubbedApplicationEvent()); + handler.add(new NoopTestApplicationEvent()); assertTrue(handler.isEmpty()); + assertFalse(client.hasInFlightRequests()); // noop does not send network request + handler.close(); + assertFalse(client.active()); } @Test @@ -61,35 +84,42 @@ public void testRunOnceBackgroundThread() { BlockingQueue applicationEventQueue = new LinkedBlockingQueue<>(); BlockingQueue backgroundEventQueue = new LinkedBlockingQueue<>(); EventHandler eventHandler = new DefaultEventHandler( - new RunOnceBackgroundThread(applicationEventQueue, backgroundEventQueue), + new RunOnceBackgroundThreadRunnable(applicationEventQueue, backgroundEventQueue), applicationEventQueue, backgroundEventQueue); - assertTrue(eventHandler.add(new StubbedApplicationEvent("hello-world"))); + assertTrue(eventHandler.add(new NoopTestApplicationEvent("hello-world"))); assertFalse(eventHandler.isEmpty()); Optional event = eventHandler.poll(); assertTrue(event.isPresent()); - assertTrue(event.get() instanceof RunOnceBackgroundThread.NoopBackgroundEvent); + assertTrue(event.get() instanceof RunOnceBackgroundThreadRunnable.NoopBackgroundEvent); assertEquals(BackgroundEvent.EventType.NOOP, event.get().type); - assertEquals("hello-world", ((RunOnceBackgroundThread.NoopBackgroundEvent) event.get()).message); + assertEquals("hello-world", ((RunOnceBackgroundThreadRunnable.NoopBackgroundEvent) event.get()).message); } - private class StubbedApplicationEvent extends ApplicationEvent { + private class NoopTestApplicationEvent extends ApplicationEvent { public final String message; - public StubbedApplicationEvent() { + public NoopTestApplicationEvent() { this(""); } - public StubbedApplicationEvent(String message) { + public NoopTestApplicationEvent(String message) { super(EventType.NOOP, false); this.message = message; } } - private class RunOnceBackgroundThread implements EventProcessor { + private ConsumerMetadata newConsumerMetadata(boolean includeInternalTopics) { + long refreshBackoffMs = 50; + long expireMs = 50000; + return new ConsumerMetadata(refreshBackoffMs, expireMs, includeInternalTopics, false, + subscriptions, new LogContext(), new ClusterResourceListeners()); + } + + private class RunOnceBackgroundThreadRunnable implements BackgroundThreadRunnable { private BlockingQueue applicationEventQueue; private BlockingQueue backgroundEventQueue; - public RunOnceBackgroundThread(BlockingQueue applicationEvents, - BlockingQueue backgroundEventQueue) { + public RunOnceBackgroundThreadRunnable(BlockingQueue applicationEvents, + BlockingQueue backgroundEventQueue) { this.applicationEventQueue = applicationEvents; this.backgroundEventQueue = backgroundEventQueue; } @@ -97,7 +127,7 @@ public RunOnceBackgroundThread(BlockingQueue applicationEvents public void run() { while (applicationEventQueue.isEmpty()) { } ApplicationEvent event = applicationEventQueue.poll(); - String message = ((StubbedApplicationEvent) event).message; + String message = ((NoopTestApplicationEvent) event).message; backgroundEventQueue.add(new NoopBackgroundEvent(message)); } From 359eda917dcfd22655770baa137d089eee8f565b Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 10:31:33 -0700 Subject: [PATCH 23/45] wip --- .../consumer/internals/DefaultBackgroundThreadRunnable.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java index cecdd79b5eb8e..27c003265c61a 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java @@ -164,9 +164,6 @@ private void setConfig() { this.heartbeatIntervalMs = config.getInt(ConsumerConfig.HEARTBEAT_INTERVAL_MS_CONFIG); } - /** - * The main processor loop. - */ @Override public void run() { try { From ebdc0d2a8a8185aa4abe21a414cc971d28834bc3 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 10:32:49 -0700 Subject: [PATCH 24/45] delete file --- .../AbstractPrototypeAsyncConsumer.java | 121 ------------------ 1 file changed, 121 deletions(-) delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java deleted file mode 100644 index 9a95168a5c19c..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java +++ /dev/null @@ -1,121 +0,0 @@ -/* - * 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 org.apache.kafka.clients.consumer.internals; - -import org.apache.kafka.clients.consumer.Consumer; -import org.apache.kafka.clients.consumer.ConsumerRecords; -import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; -import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; -import org.apache.kafka.clients.consumer.internals.events.EventHandler; -import org.apache.kafka.common.utils.Time; - -import java.time.Duration; -import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; - -/** - * This is the prototype of the consumer that uses the {@link EventHandler} to process application events. - */ -public abstract class AbstractPrototypeAsyncConsumer implements Consumer { - private final EventHandler eventHandler; - private final Time time; - - public AbstractPrototypeAsyncConsumer(final Time time, final EventHandler eventHandler) { - this.eventHandler = eventHandler; - this.time = time; - } - - /** - * poll implementation using {@link EventHandler}. - * 1. Poll for background events. If there's a fetch response event, process the record and return it. If it is - * another type of event, process it. - * 2. Send fetches if needed. - * If the timeout expires, return an empty ConsumerRecord. - * @param timeout timeout of the poll loop - * @return ConsumerRecord. It can be empty if time timeout expires. - */ - @Override - public ConsumerRecords poll(final Duration timeout) { - try { - do { - if (!eventHandler.isEmpty()) { - Optional backgroundEvent = eventHandler.poll(); - if (backgroundEvent.isPresent()) { - if (isFetchResult(backgroundEvent.get())) { - // return fetches - return processFetchResult(backgroundEvent.get()); - } - processEvent(backgroundEvent.get(), timeout); // might trigger callbacks or handle exceptions - } - } - - maybeSendFetches(); // send new fetches - } while (time.timer(timeout).notExpired()); - } catch (Exception e) { - throw new RuntimeException(e); - } - - return ConsumerRecords.empty(); - } - - abstract void processEvent(BackgroundEvent backgroundEvent, Duration timeout); - abstract boolean isFetchResult(BackgroundEvent event); - abstract ConsumerRecords processFetchResult(BackgroundEvent event); - abstract void maybeSendFetches(); - - /** - * This method sends a commit event to the EventHandler and return. - */ - @Override - public void commitAsync() { - ApplicationEvent commitEvent = new CommitApplicationEvent(); - eventHandler.add(commitEvent); - } - - /** - * This method sends a commit event to the EventHandler and waits for the event to finish. - * @param timeout max wait time for the blocking operation. - */ - @Override - public void commitSync(Duration timeout) { - CommitApplicationEvent commitEvent = new CommitApplicationEvent(); - eventHandler.add(commitEvent); - - CompletableFuture commitFuture = commitEvent.commitFuture; - try { - commitFuture.get(timeout.toMillis(), TimeUnit.MILLISECONDS); - } catch (TimeoutException e) { - throw new org.apache.kafka.common.errors.TimeoutException("timeout"); - } catch (Exception e) { - // handle exception here - throw new RuntimeException(e); - } - } - - /** - * A stubbed ApplicationEvent for demonstration purpose - */ - private class CommitApplicationEvent extends ApplicationEvent { - public CommitApplicationEvent() { - super(EventType.COMMIT, false); - } - // this is the stubbed commitAsyncEvents - CompletableFuture commitFuture = new CompletableFuture<>(); - } -} From ca1d1c07e7e0e1b61f9768bc3d6480913df9f8b9 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 11:05:02 -0700 Subject: [PATCH 25/45] tests --- .../internals/PrototypeAsyncConsumer.java | 4 + .../internals/events/ApplicationEvent.java | 5 +- .../DefaultBackgroundThreadRunnableTest.java | 85 +++++++++++++++++++ .../internals/DefaultEventHandlerTest.java | 65 +++++++------- 4 files changed, 126 insertions(+), 33 deletions(-) create mode 100644 clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnableTest.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java index 0764a27faf859..8a1ccfc2a5ed1 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java @@ -122,5 +122,9 @@ public void commitSync(Duration timeout) { private class CommitApplicationEvent extends ApplicationEvent { // this is the stubbed commitAsyncEvents CompletableFuture commitFuture = new CompletableFuture<>(); + + public CommitApplicationEvent() { + super(EventType.NOOP); + } } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java index 15d7798f9624e..25cbe678bc3ce 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java @@ -21,12 +21,11 @@ */ abstract public class ApplicationEvent { public final EventType type; - public final boolean needCoordinator; - public ApplicationEvent(EventType type, boolean needCoordinator) { + public ApplicationEvent(EventType type) { this.type = type; - this.needCoordinator = needCoordinator; } + public enum EventType { COMMIT, NOOP, diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnableTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnableTest.java new file mode 100644 index 0000000000000..996816a28a314 --- /dev/null +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnableTest.java @@ -0,0 +1,85 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals; + +import org.apache.kafka.clients.MockClient; +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.clients.consumer.OffsetResetStrategy; +import org.apache.kafka.common.internals.ClusterResourceListeners; +import org.apache.kafka.common.serialization.StringDeserializer; +import org.apache.kafka.common.utils.KafkaThread; +import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.MockTime; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Properties; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.LinkedBlockingQueue; + +import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG; +import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class DefaultBackgroundThreadRunnableTest { + private long refreshBackoffMs = 100; + private long expireMs = 1000; + private final Properties properties = new Properties(); + private MockTime time; + private SubscriptionState subscriptions; + private ConsumerMetadata metadata; + private MockClient client; + private LogContext context; + private ConsumerNetworkClient consumerClient; + + @BeforeEach + public void setup() { + this.time = new MockTime(); + this.subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); + this.metadata = new ConsumerMetadata(refreshBackoffMs, expireMs, false, false, + subscriptions, new LogContext(), new ClusterResourceListeners()); + this.client = new MockClient(time, metadata); + this.context = new LogContext(); + this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, + 100, 1000, 100); + properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + } + + @Test + public void testStartupAndTearDown() { + DefaultBackgroundThreadRunnable runnable = setupMockHandler(); + KafkaThread thread = new KafkaThread("test-thread", runnable, true); + thread.start(); + assertTrue(client.active()); + runnable.close(); + assertFalse(client.active()); + } + + private DefaultBackgroundThreadRunnable setupMockHandler() { + DefaultBackgroundThreadRunnable runnable = new DefaultBackgroundThreadRunnable( + time, + new ConsumerConfig(properties), + new LinkedBlockingDeque<>(), + new LinkedBlockingQueue<>(), + subscriptions, + metadata, + this.consumerClient); + return runnable; + } +} diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 3e02fc20793d9..1a6d0e2b0b3d7 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -43,12 +43,10 @@ public class DefaultEventHandlerTest { private final Properties properties = new Properties(); - private SubscriptionState subscriptions; @BeforeEach public void setup() { - subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); } @@ -56,8 +54,9 @@ public void setup() { @Test public void testBasicPollAndAddWithNoopEvent() { LogContext logContext = new LogContext(); + SubscriptionState subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); Time time = new MockTime(); - ConsumerMetadata metadata = newConsumerMetadata(false); + ConsumerMetadata metadata = newConsumerMetadata(false, subscriptions); MockClient client = new MockClient(time, metadata); ConsumerNetworkClient consumerClient = new ConsumerNetworkClient(logContext, client, metadata, time, 100, 1000, 100); @@ -83,6 +82,7 @@ public void testBasicPollAndAddWithNoopEvent() { public void testRunOnceBackgroundThread() { BlockingQueue applicationEventQueue = new LinkedBlockingQueue<>(); BlockingQueue backgroundEventQueue = new LinkedBlockingQueue<>(); + SubscriptionState subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); EventHandler eventHandler = new DefaultEventHandler( new RunOnceBackgroundThreadRunnable(applicationEventQueue, backgroundEventQueue), applicationEventQueue, @@ -91,24 +91,12 @@ public void testRunOnceBackgroundThread() { assertFalse(eventHandler.isEmpty()); Optional event = eventHandler.poll(); assertTrue(event.isPresent()); - assertTrue(event.get() instanceof RunOnceBackgroundThreadRunnable.NoopBackgroundEvent); + assertTrue(event.get() instanceof NoopTestBackgroundEvent); assertEquals(BackgroundEvent.EventType.NOOP, event.get().type); - assertEquals("hello-world", ((RunOnceBackgroundThreadRunnable.NoopBackgroundEvent) event.get()).message); + assertEquals("hello-world", ((NoopTestBackgroundEvent) event.get()).message); } - private class NoopTestApplicationEvent extends ApplicationEvent { - public final String message; - public NoopTestApplicationEvent() { - this(""); - } - - public NoopTestApplicationEvent(String message) { - super(EventType.NOOP, false); - this.message = message; - } - } - - private ConsumerMetadata newConsumerMetadata(boolean includeInternalTopics) { + private static ConsumerMetadata newConsumerMetadata(boolean includeInternalTopics, SubscriptionState subscriptions) { long refreshBackoffMs = 50; long expireMs = 50000; return new ConsumerMetadata(refreshBackoffMs, expireMs, includeInternalTopics, false, @@ -118,34 +106,51 @@ private ConsumerMetadata newConsumerMetadata(boolean includeInternalTopics) { private class RunOnceBackgroundThreadRunnable implements BackgroundThreadRunnable { private BlockingQueue applicationEventQueue; private BlockingQueue backgroundEventQueue; + public RunOnceBackgroundThreadRunnable(BlockingQueue applicationEvents, BlockingQueue backgroundEventQueue) { this.applicationEventQueue = applicationEvents; this.backgroundEventQueue = backgroundEventQueue; } + @Override public void run() { - while (applicationEventQueue.isEmpty()) { } + while (applicationEventQueue.isEmpty()) { + } ApplicationEvent event = applicationEventQueue.poll(); String message = ((NoopTestApplicationEvent) event).message; - backgroundEventQueue.add(new NoopBackgroundEvent(message)); + backgroundEventQueue.add(new NoopTestBackgroundEvent(message)); } @Override public void close() { } + } - class NoopBackgroundEvent extends BackgroundEvent { - public String message; - public NoopBackgroundEvent(String message) { - super(EventType.NOOP); - this.message = message; - } + private class NoopTestApplicationEvent extends ApplicationEvent { + public final String message; - @Override - public String toString() { - return type + ":" + message; - } + public NoopTestApplicationEvent() { + this(""); + } + + public NoopTestApplicationEvent(String message) { + super(EventType.NOOP); + this.message = message; + } + } + + private class NoopTestBackgroundEvent extends BackgroundEvent { + public String message; + + public NoopTestBackgroundEvent(String message) { + super(EventType.NOOP); + this.message = message; + } + + @Override + public String toString() { + return type + ":" + message; } } } From 6f1f8e572d63d40921a414ffac3f3b30a74e007a Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 12:38:43 -0700 Subject: [PATCH 26/45] extra space --- .../clients/consumer/internals/DefaultEventHandlerTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 1a6d0e2b0b3d7..62829b48b4776 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -44,7 +44,6 @@ public class DefaultEventHandlerTest { private final Properties properties = new Properties(); - @BeforeEach public void setup() { properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); From 14d9f523bdbeddcd60c9153e94454742300a46ee Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 12:39:50 -0700 Subject: [PATCH 27/45] spaces --- .../consumer/internals/DefaultBackgroundThreadRunnable.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java index 27c003265c61a..90aea2c350695 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java @@ -14,7 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.apache.kafka.clients.consumer.internals; import org.apache.kafka.clients.ApiVersions; @@ -233,4 +232,4 @@ private ConsumerMetadata bootstrapMetadata(ClusterResourceListeners clusterResou metadata.bootstrap(addresses); return metadata; } -} \ No newline at end of file +} From 5855d742f2fa361d414394fc9255fc411261b595 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 15:55:44 -0700 Subject: [PATCH 28/45] tests seem flakey --- .../internals/DefaultEventHandlerTest.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 62829b48b4776..9399b3126a67e 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -29,11 +29,13 @@ import org.apache.kafka.common.utils.Time; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import java.util.Optional; import java.util.Properties; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; +import java.util.function.Supplier; import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG; import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG; @@ -78,16 +80,16 @@ public void testBasicPollAndAddWithNoopEvent() { } @Test - public void testRunOnceBackgroundThread() { + @Timeout(1) + public void testRunOnceBackgroundThread() throws InterruptedException { BlockingQueue applicationEventQueue = new LinkedBlockingQueue<>(); BlockingQueue backgroundEventQueue = new LinkedBlockingQueue<>(); - SubscriptionState subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); EventHandler eventHandler = new DefaultEventHandler( new RunOnceBackgroundThreadRunnable(applicationEventQueue, backgroundEventQueue), applicationEventQueue, backgroundEventQueue); assertTrue(eventHandler.add(new NoopTestApplicationEvent("hello-world"))); - assertFalse(eventHandler.isEmpty()); + runUntil(() -> !eventHandler.isEmpty(), 1000); Optional event = eventHandler.poll(); assertTrue(event.isPresent()); assertTrue(event.get() instanceof NoopTestBackgroundEvent); @@ -152,4 +154,16 @@ public String toString() { return type + ":" + message; } } + + void runUntil( + Supplier condition, + int timeoutMs + ) throws InterruptedException { + int tries = 0; + while (!condition.get()) { + tries++; + this.wait(timeoutMs); + } + assertTrue(condition.get(), "Condition not satisfied after " + timeoutMs + "ms"); + } } From 0cbf1f9cd91ec926dd5fe2bed2944964aab9af04 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 16:33:00 -0700 Subject: [PATCH 29/45] PR comments --- .../consumer/internals/BackgroundThreadRunnable.java | 4 ++-- .../internals/DefaultBackgroundThreadRunnable.java | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java index a327055aebcd6..c27bdd0311488 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java @@ -19,8 +19,8 @@ import java.io.Closeable; /** - * The {@code EventHandler} constructs a thread that runs {@code BackgroundThreadRunnable} to handle network requests - * and responses. + * The {@link org.apache.kafka.clients.consumer.internals.events.EventHandler} constructs a thread that runs + * {@code BackgroundThreadRunnable} to handle network requests and responses. */ public interface BackgroundThreadRunnable extends Runnable, Closeable { } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java index 90aea2c350695..528434f7ef6f2 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java @@ -96,6 +96,7 @@ public DefaultBackgroundThreadRunnable(Time time, ClusterResourceListeners clusterResourceListeners, Sensor fetcherThrottleTimeSensor) { try { + this.time = time; this.log = logContext.logger(DefaultBackgroundThreadRunnable.class); this.applicationEventQueue = applicationEventQueue; @@ -109,7 +110,7 @@ public DefaultBackgroundThreadRunnable(Time time, ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext); NetworkClient netClient = new NetworkClient( new Selector(config.getLong(ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), metrics, time, METRIC_GRP_PREFIX, channelBuilder, logContext), - this.metadata, + metadata, clientId, 100, // a fixed large enough value will suffice for max in-flight requests config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG), @@ -148,6 +149,7 @@ public DefaultBackgroundThreadRunnable(Time time, ConsumerNetworkClient client) { this.time = time; this.config = config; + setConfig(); this.log = LoggerFactory.getLogger(getClass()); this.applicationEventQueue = applicationEventQueue; this.backgroundEventQueue = backgroundEventQueue; @@ -209,7 +211,7 @@ public boolean maybeConsumeInflightEvent(ApplicationEvent event) { return true; default: inflightEvent = Optional.empty(); - log.info("unsupported event type: {}", event.type); + log.warn("unsupported event type: {}", event.type); } return false; } @@ -218,6 +220,7 @@ public boolean maybeConsumeInflightEvent(ApplicationEvent event) { public void close() { this.running = false; Utils.closeQuietly(networkClient, "consumer network client"); + Utils.closeQuietly(metadata, "consumer network client"); } private ConsumerMetadata bootstrapMetadata(ClusterResourceListeners clusterResourceListeners, LogContext logContext) { From 6cfa912a301dbc036390e3b32db000b37569b22a Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 3 Oct 2022 16:39:17 -0700 Subject: [PATCH 30/45] cleaned up PR blank line reduce logic fixed a few tests documentation fix the logic wip --- .../DefaultBackgroundThreadRunnable.java | 44 +++++++---- .../internals/DefaultEventHandler.java | 8 +- .../internals/NoopBackgroundEvent.java | 36 +++++++++ .../events/NoopApplicationEvent.java | 34 +++++++++ .../internals/DefaultEventHandlerTest.java | 76 +++++-------------- 5 files changed, 124 insertions(+), 74 deletions(-) create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java create mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java index 528434f7ef6f2..2a181a304128d 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java @@ -23,6 +23,7 @@ import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; +import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.KafkaException; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.metrics.Metrics; @@ -41,8 +42,10 @@ import java.util.concurrent.BlockingQueue; /** - * Lives inside of the {@code DefaultEventHandler}, and consumes {@code ApplicationEvent} and produces - * {@code BackgroundEvent}. + * The background process of the {@code DefaultEventHandler} that consumes {@code ApplicationEvent} and produces + * {@code BackgroundEvent}. It owns the network client and handles all the network IO to the brokers. + * + * It holds a reference to the {@link SubscriptionState}, which is initialized by the polling thread. */ public class DefaultBackgroundThreadRunnable implements BackgroundThreadRunnable { private static final String CLIENT_ID_METRIC_TAG = "client-id"; @@ -53,16 +56,16 @@ public class DefaultBackgroundThreadRunnable implements BackgroundThreadRunnable private final BlockingQueue applicationEventQueue; private final BlockingQueue backgroundEventQueue; private final ConsumerNetworkClient networkClient; - - private final ConsumerConfig config; - private final Metrics metrics; private final SubscriptionState subscriptions; private final ConsumerMetadata metadata; + private final Metrics metrics; + private final ConsumerConfig config; + private String clientId; private long retryBackoffMs; private int heartbeatIntervalMs; - private boolean running = false; - private Optional inflightEvent; + private boolean running; + private Optional inflightEvent = Optional.empty(); public DefaultBackgroundThreadRunnable(ConsumerConfig config, LogContext logContext, @@ -96,7 +99,6 @@ public DefaultBackgroundThreadRunnable(Time time, ClusterResourceListeners clusterResourceListeners, Sensor fetcherThrottleTimeSensor) { try { - this.time = time; this.log = logContext.logger(DefaultBackgroundThreadRunnable.class); this.applicationEventQueue = applicationEventQueue; @@ -104,7 +106,7 @@ public DefaultBackgroundThreadRunnable(Time time, this.config = config; setConfig(); this.inflightEvent = Optional.empty(); - this.subscriptions = subscriptions; + this.subscriptions = subscriptions; // subscriptionState is initialized in the polling thread and passed here. this.metrics = metrics; this.metadata = bootstrapMetadata(clusterResourceListeners, logContext); ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext); @@ -133,8 +135,10 @@ public DefaultBackgroundThreadRunnable(Time time, retryBackoffMs, config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), heartbeatIntervalMs); + this.running = true; } catch (Exception e) { // now propagate the exception + close(); throw new KafkaException("Failed to construct background processor", e); } } @@ -157,6 +161,7 @@ public DefaultBackgroundThreadRunnable(Time time, this.metadata = metadata; this.networkClient = client; this.metrics = new Metrics(); + this.running = true; } private void setConfig() { @@ -168,15 +173,15 @@ private void setConfig() { @Override public void run() { try { - log.debug("DefaultBackgroundThreadRunnable started"); + log.debug("{} started", getClass()); while (running) { pollOnce(); - this.wait(retryBackoffMs); + time.sleep(retryBackoffMs); } } catch (Exception e) { // TODO: Define fine grain exceptions here } finally { - log.debug("DefaultBackgroundThreadRunnable closed"); + log.debug("{} closed", getClass()); } } @@ -193,7 +198,7 @@ void pollOnce() { } public Optional maybePollEvent() { - if (this.inflightEvent.isPresent()) { + if (this.inflightEvent.isPresent() || this.applicationEventQueue.isEmpty()) { return this.inflightEvent; } return Optional.ofNullable(this.applicationEventQueue.poll()); @@ -205,15 +210,24 @@ public Optional maybePollEvent() { * @return true when successfully consumed the event. */ public boolean maybeConsumeInflightEvent(ApplicationEvent event) { + log.debug("try consuming event: {}", Optional.ofNullable(event)); switch (event.type) { case NOOP: - log.info("consuming a NOOP event"); + process((NoopApplicationEvent) event); return true; default: inflightEvent = Optional.empty(); log.warn("unsupported event type: {}", event.type); + return true; } - return false; + } + + /** + * Processes {@link NoopApplicationEvent} and equeue a {@link NoopBackgroundEvent}. + * @param event a {@link NoopApplicationEvent} + */ + private void process(NoopApplicationEvent event) { + backgroundEventQueue.add(new NoopBackgroundEvent(event.message)); } @Override diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index e66ec164c62e7..6ccaefba39a52 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -84,17 +84,17 @@ public DefaultEventHandler(ConsumerConfig config, LogContext logcontext, SubscriptionState subscriptionState, ConsumerMetadata metadata, ConsumerNetworkClient networkClient) { + this.applicationEventQueue = applicationEventQueue; + this.backgroundEventQueue = backgroundEventQueue; this.runnable = new DefaultBackgroundThreadRunnable( time, config, - applicationEventQueue, - backgroundEventQueue, + this.applicationEventQueue, + this.backgroundEventQueue, subscriptionState, metadata, networkClient); this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true); - this.applicationEventQueue = applicationEventQueue; - this.backgroundEventQueue = backgroundEventQueue; backgroundThread.start(); } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java new file mode 100644 index 0000000000000..bbedbcace7652 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java @@ -0,0 +1,36 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals; + +import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; + +/** + * Noop event. Intentionally left it here for demonstration purpose. + */ +public class NoopBackgroundEvent extends BackgroundEvent { + public final String message; + + public NoopBackgroundEvent(String message) { + super(EventType.NOOP); + this.message = message; + } + + @Override + public String toString() { + return getClass() + "_" + this.message; + } +} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java new file mode 100644 index 0000000000000..1a56be8fa5300 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java @@ -0,0 +1,34 @@ +/* + * 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 org.apache.kafka.clients.consumer.internals.events; + +/** + * The event is NoOp. This is intentionally left here for demonstration purpose. + */ +public class NoopApplicationEvent extends ApplicationEvent { + public final String message; + + public NoopApplicationEvent(String message) { + super(EventType.NOOP); + this.message = message; + } + + @Override + public String toString() { + return getClass() + "_" + this.message; + } +} diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 9399b3126a67e..734a0f7e8a194 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -22,6 +22,7 @@ import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.serialization.StringDeserializer; import org.apache.kafka.common.utils.LogContext; @@ -35,9 +36,9 @@ import java.util.Properties; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; -import java.util.function.Supplier; import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG; +import static org.apache.kafka.clients.consumer.ConsumerConfig.RETRY_BACKOFF_MS_CONFIG; import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -50,30 +51,35 @@ public class DefaultEventHandlerTest { public void setup() { properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + properties.put(RETRY_BACKOFF_MS_CONFIG, "100"); } @Test + @Timeout(1) public void testBasicPollAndAddWithNoopEvent() { + Time time = new MockTime(1); LogContext logContext = new LogContext(); SubscriptionState subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); - Time time = new MockTime(); ConsumerMetadata metadata = newConsumerMetadata(false, subscriptions); MockClient client = new MockClient(time, metadata); ConsumerNetworkClient consumerClient = new ConsumerNetworkClient(logContext, client, metadata, time, 100, 1000, 100); + BlockingQueue aq = new LinkedBlockingQueue<>(); + BlockingQueue bq = new LinkedBlockingQueue<>(); DefaultEventHandler handler = new DefaultEventHandler( time, new ConsumerConfig(properties), - new LinkedBlockingQueue<>(), - new LinkedBlockingQueue<>(), + aq, bq, subscriptions, metadata, consumerClient); assertTrue(client.active()); assertTrue(handler.isEmpty()); - assertTrue(!handler.poll().isPresent()); - handler.add(new NoopTestApplicationEvent()); - assertTrue(handler.isEmpty()); + handler.add(new NoopApplicationEvent("testBasicPollAndAddWithNoopEvent")); + while (handler.isEmpty()) { + time.sleep(100); + } + assertTrue(handler.poll().get() instanceof NoopBackgroundEvent); assertFalse(client.hasInFlightRequests()); // noop does not send network request handler.close(); assertFalse(client.active()); @@ -81,20 +87,20 @@ public void testBasicPollAndAddWithNoopEvent() { @Test @Timeout(1) - public void testRunOnceBackgroundThread() throws InterruptedException { + public void testRunOnceBackgroundThread() { BlockingQueue applicationEventQueue = new LinkedBlockingQueue<>(); BlockingQueue backgroundEventQueue = new LinkedBlockingQueue<>(); EventHandler eventHandler = new DefaultEventHandler( new RunOnceBackgroundThreadRunnable(applicationEventQueue, backgroundEventQueue), applicationEventQueue, backgroundEventQueue); - assertTrue(eventHandler.add(new NoopTestApplicationEvent("hello-world"))); - runUntil(() -> !eventHandler.isEmpty(), 1000); + assertTrue(eventHandler.add(new NoopApplicationEvent("hello-world"))); + while (eventHandler.isEmpty()) { } Optional event = eventHandler.poll(); assertTrue(event.isPresent()); - assertTrue(event.get() instanceof NoopTestBackgroundEvent); + assertTrue(event.get() instanceof NoopBackgroundEvent); assertEquals(BackgroundEvent.EventType.NOOP, event.get().type); - assertEquals("hello-world", ((NoopTestBackgroundEvent) event.get()).message); + assertEquals("hello-world", ((NoopBackgroundEvent) event.get()).message); } private static ConsumerMetadata newConsumerMetadata(boolean includeInternalTopics, SubscriptionState subscriptions) { @@ -116,54 +122,14 @@ public RunOnceBackgroundThreadRunnable(BlockingQueue applicati @Override public void run() { - while (applicationEventQueue.isEmpty()) { - } + while (applicationEventQueue.isEmpty()) { } ApplicationEvent event = applicationEventQueue.poll(); - String message = ((NoopTestApplicationEvent) event).message; - backgroundEventQueue.add(new NoopTestBackgroundEvent(message)); + String message = ((NoopApplicationEvent) event).message; + backgroundEventQueue.add(new NoopBackgroundEvent(message)); } @Override public void close() { } } - - private class NoopTestApplicationEvent extends ApplicationEvent { - public final String message; - - public NoopTestApplicationEvent() { - this(""); - } - - public NoopTestApplicationEvent(String message) { - super(EventType.NOOP); - this.message = message; - } - } - - private class NoopTestBackgroundEvent extends BackgroundEvent { - public String message; - - public NoopTestBackgroundEvent(String message) { - super(EventType.NOOP); - this.message = message; - } - - @Override - public String toString() { - return type + ":" + message; - } - } - - void runUntil( - Supplier condition, - int timeoutMs - ) throws InterruptedException { - int tries = 0; - while (!condition.get()) { - tries++; - this.wait(timeoutMs); - } - assertTrue(condition.get(), "Condition not satisfied after " + timeoutMs + "ms"); - } } From c3d3d037debdc2709d8fa8361df289bae7a54b2c Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 4 Oct 2022 11:59:17 -0700 Subject: [PATCH 31/45] exception handling --- .../internals/DefaultBackgroundThreadRunnable.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java index 2a181a304128d..8395e0cad873f 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java @@ -25,6 +25,7 @@ import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.KafkaException; +import org.apache.kafka.common.errors.InterruptException; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.metrics.Sensor; @@ -40,6 +41,7 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutionException; /** * The background process of the {@code DefaultEventHandler} that consumes {@code ApplicationEvent} and produces @@ -178,6 +180,8 @@ public void run() { pollOnce(); time.sleep(retryBackoffMs); } + } catch (InterruptException e) { + throw new RuntimeException(e); } catch (Exception e) { // TODO: Define fine grain exceptions here } finally { @@ -223,7 +227,8 @@ public boolean maybeConsumeInflightEvent(ApplicationEvent event) { } /** - * Processes {@link NoopApplicationEvent} and equeue a {@link NoopBackgroundEvent}. + * Processes {@link NoopApplicationEvent} and equeue a {@link NoopBackgroundEvent}. This is intentionally left here + * for demonstration purpose. * @param event a {@link NoopApplicationEvent} */ private void process(NoopApplicationEvent event) { From 7669b0acb9476c08cd07a1ecd7e9973e6ebdc898 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 4 Oct 2022 13:19:23 -0700 Subject: [PATCH 32/45] clean up unused import --- .../consumer/internals/DefaultBackgroundThreadRunnable.java | 1 - 1 file changed, 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java index 8395e0cad873f..c7819a528af6b 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java @@ -41,7 +41,6 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.BlockingQueue; -import java.util.concurrent.ExecutionException; /** * The background process of the {@code DefaultEventHandler} that consumes {@code ApplicationEvent} and produces From 2d66bda8f5106fdace4bbf05b41acb3e32306ad7 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Thu, 6 Oct 2022 14:30:23 -0700 Subject: [PATCH 33/45] More documentation and better exception handling --- .../internals/BackgroundThreadRunnable.java | 3 +- .../DefaultBackgroundThreadRunnable.java | 52 ++++++++++++++----- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java index c27bdd0311488..3ce01a692b554 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java @@ -19,8 +19,7 @@ import java.io.Closeable; /** - * The {@link org.apache.kafka.clients.consumer.internals.events.EventHandler} constructs a thread that runs - * {@code BackgroundThreadRunnable} to handle network requests and responses. + * Background thread runnable that handles network IO such as fetching and committing. */ public interface BackgroundThreadRunnable extends Runnable, Closeable { } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java index c7819a528af6b..955b3762343ab 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java @@ -41,15 +41,17 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.atomic.AtomicReference; /** - * The background process of the {@code DefaultEventHandler} that consumes {@code ApplicationEvent} and produces - * {@code BackgroundEvent}. It owns the network client and handles all the network IO to the brokers. + * Background thread runnable that consumes {@code ApplicationEvent} and + * produces {@code BackgroundEvent}. It uses an event loop to consume and + * produce events, and poll the network client to handle network IO. * - * It holds a reference to the {@link SubscriptionState}, which is initialized by the polling thread. + * It holds a reference to the {@link SubscriptionState}, which is + * initialized by the polling thread. */ public class DefaultBackgroundThreadRunnable implements BackgroundThreadRunnable { - private static final String CLIENT_ID_METRIC_TAG = "client-id"; private static final String METRIC_GRP_PREFIX = "consumer"; private final Time time; @@ -67,6 +69,8 @@ public class DefaultBackgroundThreadRunnable implements BackgroundThreadRunnable private int heartbeatIntervalMs; private boolean running; private Optional inflightEvent = Optional.empty(); + private AtomicReference> exception = + new AtomicReference<>(Optional.empty()); public DefaultBackgroundThreadRunnable(ConsumerConfig config, LogContext logContext, @@ -107,15 +111,24 @@ public DefaultBackgroundThreadRunnable(Time time, this.config = config; setConfig(); this.inflightEvent = Optional.empty(); - this.subscriptions = subscriptions; // subscriptionState is initialized in the polling thread and passed here. + // subscriptionState is initialized in the polling thread + this.subscriptions = subscriptions; this.metrics = metrics; this.metadata = bootstrapMetadata(clusterResourceListeners, logContext); ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext); + Selector selector = new Selector(config.getLong( + ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), + metrics, + time, + METRIC_GRP_PREFIX, + channelBuilder, + logContext); NetworkClient netClient = new NetworkClient( - new Selector(config.getLong(ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), metrics, time, METRIC_GRP_PREFIX, channelBuilder, logContext), + selector, metadata, clientId, - 100, // a fixed large enough value will suffice for max in-flight requests + 100, // a fixed large enough value will suffice for max + // in-flight requests config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG), config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MAX_MS_CONFIG), config.getInt(ConsumerConfig.SEND_BUFFER_CONFIG), @@ -176,14 +189,20 @@ public void run() { try { log.debug("{} started", getClass()); while (running) { - pollOnce(); + runOnce(); time.sleep(retryBackoffMs); } } catch (InterruptException e) { - throw new RuntimeException(e); - } catch (Exception e) { - // TODO: Define fine grain exceptions here + log.error("The background thread has been interrupted"); + this.exception.set(Optional.of(new RuntimeException(e))); + } catch (Throwable t) { + log.error("The background failed due to unexpected error", t); + if (t instanceof RuntimeException) + this.exception.set(Optional.of((RuntimeException) t)); + else + this.exception.set(Optional.of(new RuntimeException(t))); } finally { + close(); log.debug("{} closed", getClass()); } } @@ -191,8 +210,9 @@ public void run() { /** * Process event from a single poll */ - void pollOnce() { + void runOnce() { this.inflightEvent = maybePollEvent(); + log.debug("processing applicatoin event: {}", this.inflightEvent); if (this.inflightEvent.isPresent() && maybeConsumeInflightEvent(this.inflightEvent.get())) { // clear inflight event upon successful consumption this.inflightEvent = Optional.empty(); @@ -234,6 +254,14 @@ private void process(NoopApplicationEvent event) { backgroundEventQueue.add(new NoopBackgroundEvent(event.message)); } + public boolean isRunning() { + return this.running; + } + + public Optional exception() { + return this.exception.get(); + } + @Override public void close() { this.running = false; From 3a52b131708bc45078fa2bfd3f775dc72ea76316 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 10 Oct 2022 09:44:56 -0700 Subject: [PATCH 34/45] refactor based on PR comment refactor PR comment --- .../internals/BackgroundThreadRunnable.java | 25 --- ...able.java => DefaultBackgroundThread.java} | 151 +++++------------- .../internals/DefaultEventHandler.java | 96 ++++++----- .../internals/events/EventHandler.java | 9 ++ ....java => DefaultBackgroundThreadTest.java} | 80 +++++++--- .../internals/DefaultEventHandlerTest.java | 12 +- 6 files changed, 164 insertions(+), 209 deletions(-) delete mode 100644 clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java rename clients/src/main/java/org/apache/kafka/clients/consumer/internals/{DefaultBackgroundThreadRunnable.java => DefaultBackgroundThread.java} (51%) rename clients/src/test/java/org/apache/kafka/clients/consumer/internals/{DefaultBackgroundThreadRunnableTest.java => DefaultBackgroundThreadTest.java} (50%) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java deleted file mode 100644 index 3ce01a692b554..0000000000000 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/BackgroundThreadRunnable.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * 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 org.apache.kafka.clients.consumer.internals; - -import java.io.Closeable; - -/** - * Background thread runnable that handles network IO such as fetching and committing. - */ -public interface BackgroundThreadRunnable extends Runnable, Closeable { -} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java similarity index 51% rename from clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java rename to clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index 955b3762343ab..1fe8071b76bac 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnable.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -16,29 +16,20 @@ */ package org.apache.kafka.clients.consumer.internals; -import org.apache.kafka.clients.ApiVersions; -import org.apache.kafka.clients.ClientUtils; import org.apache.kafka.clients.CommonClientConfigs; -import org.apache.kafka.clients.NetworkClient; import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.KafkaException; import org.apache.kafka.common.errors.InterruptException; -import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.metrics.Metrics; -import org.apache.kafka.common.metrics.Sensor; -import org.apache.kafka.common.network.ChannelBuilder; -import org.apache.kafka.common.network.Selector; +import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.Time; import org.apache.kafka.common.utils.Utils; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import java.net.InetSocketAddress; -import java.util.List; import java.util.Optional; import java.util.concurrent.BlockingQueue; import java.util.concurrent.atomic.AtomicReference; @@ -51,9 +42,9 @@ * It holds a reference to the {@link SubscriptionState}, which is * initialized by the polling thread. */ -public class DefaultBackgroundThreadRunnable implements BackgroundThreadRunnable { - private static final String METRIC_GRP_PREFIX = "consumer"; - +public class DefaultBackgroundThread extends KafkaThread { + private static final String BACKGROUND_THREAD_NAME = + "consumer_background_thread"; private final Time time; private final Logger log; private final BlockingQueue applicationEventQueue; @@ -72,40 +63,38 @@ public class DefaultBackgroundThreadRunnable implements BackgroundThreadRunnable private AtomicReference> exception = new AtomicReference<>(Optional.empty()); - public DefaultBackgroundThreadRunnable(ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue, - SubscriptionState subscriptions, - ApiVersions apiVersions, - Metrics metrics, - ClusterResourceListeners clusterResourceListeners, - Sensor fetcherThrottleTimeSensor) { + public DefaultBackgroundThread(ConsumerConfig config, + LogContext logContext, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue, + SubscriptionState subscriptions, + ConsumerMetadata metadata, + ConsumerNetworkClient networkClient, + Metrics metrics) { this(Time.SYSTEM, config, logContext, applicationEventQueue, backgroundEventQueue, subscriptions, - apiVersions, - metrics, - clusterResourceListeners, - fetcherThrottleTimeSensor); + metadata, + networkClient, + metrics); } - public DefaultBackgroundThreadRunnable(Time time, - ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue, - SubscriptionState subscriptions, - ApiVersions apiVersions, - Metrics metrics, - ClusterResourceListeners clusterResourceListeners, - Sensor fetcherThrottleTimeSensor) { + public DefaultBackgroundThread(Time time, + ConsumerConfig config, + LogContext logContext, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue, + SubscriptionState subscriptions, + ConsumerMetadata metadata, + ConsumerNetworkClient networkClient, + Metrics metrics) { + super(BACKGROUND_THREAD_NAME, true); try { this.time = time; - this.log = logContext.logger(DefaultBackgroundThreadRunnable.class); + this.log = logContext.logger(DefaultBackgroundThread.class); this.applicationEventQueue = applicationEventQueue; this.backgroundEventQueue = backgroundEventQueue; this.config = config; @@ -113,42 +102,9 @@ public DefaultBackgroundThreadRunnable(Time time, this.inflightEvent = Optional.empty(); // subscriptionState is initialized in the polling thread this.subscriptions = subscriptions; + this.metadata = metadata; + this.networkClient = networkClient; this.metrics = metrics; - this.metadata = bootstrapMetadata(clusterResourceListeners, logContext); - ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext); - Selector selector = new Selector(config.getLong( - ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), - metrics, - time, - METRIC_GRP_PREFIX, - channelBuilder, - logContext); - NetworkClient netClient = new NetworkClient( - selector, - metadata, - clientId, - 100, // a fixed large enough value will suffice for max - // in-flight requests - config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG), - config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MAX_MS_CONFIG), - config.getInt(ConsumerConfig.SEND_BUFFER_CONFIG), - config.getInt(ConsumerConfig.RECEIVE_BUFFER_CONFIG), - config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), - config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_CONFIG), - config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG), - time, - true, - apiVersions, - fetcherThrottleTimeSensor, - logContext); - this.networkClient = new ConsumerNetworkClient( - logContext, - netClient, - metadata, - time, - retryBackoffMs, - config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), - heartbeatIntervalMs); this.running = true; } catch (Exception e) { // now propagate the exception @@ -157,27 +113,6 @@ public DefaultBackgroundThreadRunnable(Time time, } } - // VisibleForTesting - DefaultBackgroundThreadRunnable(Time time, - ConsumerConfig config, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue, - SubscriptionState subscriptions, - ConsumerMetadata metadata, - ConsumerNetworkClient client) { - this.time = time; - this.config = config; - setConfig(); - this.log = LoggerFactory.getLogger(getClass()); - this.applicationEventQueue = applicationEventQueue; - this.backgroundEventQueue = backgroundEventQueue; - this.subscriptions = subscriptions; - this.metadata = metadata; - this.networkClient = client; - this.metrics = new Metrics(); - this.running = true; - } - private void setConfig() { this.retryBackoffMs = this.config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG); this.clientId = config.getString(CommonClientConfigs.CLIENT_ID_CONFIG); @@ -195,8 +130,10 @@ public void run() { } catch (InterruptException e) { log.error("The background thread has been interrupted"); this.exception.set(Optional.of(new RuntimeException(e))); + Thread.interrupted(); } catch (Throwable t) { - log.error("The background failed due to unexpected error", t); + log.error("The background thread failed due to unexpected error", + t); if (t instanceof RuntimeException) this.exception.set(Optional.of((RuntimeException) t)); else @@ -217,7 +154,8 @@ void runOnce() { // clear inflight event upon successful consumption this.inflightEvent = Optional.empty(); } - networkClient.pollNoWakeup(); + System.out.println(retryBackoffMs); + networkClient.poll(time.timer(this.retryBackoffMs)); } public Optional maybePollEvent() { @@ -246,8 +184,9 @@ public boolean maybeConsumeInflightEvent(ApplicationEvent event) { } /** - * Processes {@link NoopApplicationEvent} and equeue a {@link NoopBackgroundEvent}. This is intentionally left here - * for demonstration purpose. + * Processes {@link NoopApplicationEvent} and equeue a + * {@link NoopBackgroundEvent}. This is intentionally left here for + * demonstration purpose. * @param event a {@link NoopApplicationEvent} */ private void process(NoopApplicationEvent event) { @@ -258,27 +197,11 @@ public boolean isRunning() { return this.running; } - public Optional exception() { - return this.exception.get(); - } - - @Override public void close() { this.running = false; Utils.closeQuietly(networkClient, "consumer network client"); Utils.closeQuietly(metadata, "consumer network client"); } - private ConsumerMetadata bootstrapMetadata(ClusterResourceListeners clusterResourceListeners, LogContext logContext) { - ConsumerMetadata metadata = new ConsumerMetadata(retryBackoffMs, - config.getLong(ConsumerConfig.METADATA_MAX_AGE_CONFIG), - !config.getBoolean(ConsumerConfig.EXCLUDE_INTERNAL_TOPICS_CONFIG), - config.getBoolean(ConsumerConfig.ALLOW_AUTO_CREATE_TOPICS_CONFIG), - this.subscriptions, - logContext, clusterResourceListeners); - List addresses = ClientUtils.parseAndValidateAddresses( - config.getList(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG), config.getString(ConsumerConfig.CLIENT_DNS_LOOKUP_CONFIG)); - metadata.bootstrap(addresses); - return metadata; - } + } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 6ccaefba39a52..cc0ae053ab09c 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -16,69 +16,38 @@ */ package org.apache.kafka.clients.consumer.internals; -import org.apache.kafka.clients.ApiVersions; +import org.apache.kafka.clients.ClientUtils; import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import org.apache.kafka.common.errors.InterruptException; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.metrics.Metrics; -import org.apache.kafka.common.metrics.Sensor; import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.Time; +import java.net.InetSocketAddress; +import java.time.Duration; +import java.util.List; import java.util.Optional; import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; /** * An {@code EventHandler} that uses a single background thread to consume {@code ApplicationEvent} and produce * {@code BackgroundEvent} from the {@ConsumerBackgroundThread}. */ public class DefaultEventHandler implements EventHandler { + private static final String METRIC_GRP_PREFIX = "consumer"; private final BlockingQueue applicationEventQueue; private final BlockingQueue backgroundEventQueue; - private final BackgroundThreadRunnable runnable; private final KafkaThread backgroundThread; - public DefaultEventHandler(ConsumerConfig config, LogContext logcontext, - SubscriptionState subscriptionState, - Metrics metrics, - ClusterResourceListeners clusterResourceListeners, - Sensor fetcherThrottleTimeSensor, - ApiVersions apiVersions) { - this.applicationEventQueue = new LinkedBlockingQueue<>(); - this.backgroundEventQueue = new LinkedBlockingQueue<>(); - this.runnable = new DefaultBackgroundThreadRunnable( - config, - logcontext, - applicationEventQueue, - backgroundEventQueue, - subscriptionState, - apiVersions, - metrics, - clusterResourceListeners, - fetcherThrottleTimeSensor); - this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true); - backgroundThread.start(); - - } - - // VisibleForTesting - DefaultEventHandler(BackgroundThreadRunnable runnable, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue) { - this.runnable = runnable; - this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true); - this.applicationEventQueue = applicationEventQueue; - this.backgroundEventQueue = backgroundEventQueue; - backgroundThread.start(); - } - - // VisibleForTesting - DefaultEventHandler(Time time, + public DefaultEventHandler(Time time, ConsumerConfig config, + LogContext logContext, BlockingQueue applicationEventQueue, BlockingQueue backgroundEventQueue, SubscriptionState subscriptionState, @@ -86,15 +55,26 @@ public DefaultEventHandler(ConsumerConfig config, LogContext logcontext, ConsumerNetworkClient networkClient) { this.applicationEventQueue = applicationEventQueue; this.backgroundEventQueue = backgroundEventQueue; - this.runnable = new DefaultBackgroundThreadRunnable( + this.backgroundThread = new DefaultBackgroundThread( time, config, + logContext, this.applicationEventQueue, this.backgroundEventQueue, subscriptionState, metadata, - networkClient); - this.backgroundThread = new KafkaThread("consumer_background_thread", runnable, true); + networkClient, + new Metrics(time)); + backgroundThread.start(); + } + + // VisibleForTesting + DefaultEventHandler(KafkaThread backgroundThread, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue) { + this.backgroundThread = backgroundThread; + this.applicationEventQueue = applicationEventQueue; + this.backgroundEventQueue = backgroundEventQueue; backgroundThread.start(); } @@ -103,6 +83,16 @@ public Optional poll() { return Optional.ofNullable(backgroundEventQueue.poll()); } + @Override + public Optional poll(Duration timeout) { + try { + return Optional.ofNullable(backgroundEventQueue.poll(timeout.toMillis(), + TimeUnit.MILLISECONDS)); + } catch (InterruptedException e) { + throw new InterruptException(e); + } + } + @Override public boolean isEmpty() { return backgroundEventQueue.isEmpty(); @@ -118,9 +108,27 @@ public boolean add(ApplicationEvent event) { } } + private ConsumerMetadata bootstrapMetadata( + LogContext logContext, + ClusterResourceListeners clusterResourceListeners, + ConsumerConfig config, + SubscriptionState subscriptions) { + ConsumerMetadata metadata = new ConsumerMetadata( + config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), + config.getLong(ConsumerConfig.METADATA_MAX_AGE_CONFIG), + !config.getBoolean(ConsumerConfig.EXCLUDE_INTERNAL_TOPICS_CONFIG), + config.getBoolean(ConsumerConfig.ALLOW_AUTO_CREATE_TOPICS_CONFIG), + subscriptions, + logContext, clusterResourceListeners); + List addresses = ClientUtils.parseAndValidateAddresses( + config.getList(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG), config.getString(ConsumerConfig.CLIENT_DNS_LOOKUP_CONFIG)); + metadata.bootstrap(addresses); + return metadata; + } + public void close() { try { - this.runnable.close(); + // this.backgroundThread.interrupt(); // close logic } catch (Exception e) { throw new RuntimeException(e); diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java index c07cc33cf7353..cc648a483f788 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java @@ -16,6 +16,7 @@ */ package org.apache.kafka.clients.consumer.internals.events; +import java.time.Duration; import java.util.Optional; /** @@ -29,6 +30,14 @@ public interface EventHandler { */ Optional poll(); + /** + * Retrieves and removes a {@link BackgroundEvent}, waiting up to the {@code + * timeout} for an event to become available; + * @return an Optional of {@link BackgroundEvent} if the value is present + * . Otherwise, an empty Optional. + */ + Optional poll(Duration timeout); + /** * Check whether there are pending {@code BackgroundEvent} await to be consumed. * @return true if there are no pending event diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnableTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java similarity index 50% rename from clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnableTest.java rename to clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 996816a28a314..ba8e8f43b2188 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadRunnableTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -18,27 +18,34 @@ import org.apache.kafka.clients.MockClient; import org.apache.kafka.clients.consumer.ConsumerConfig; -import org.apache.kafka.clients.consumer.OffsetResetStrategy; -import org.apache.kafka.common.internals.ClusterResourceListeners; +import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; +import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; +import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; +import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.serialization.StringDeserializer; import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.MockTime; +import org.apache.kafka.common.utils.Timer; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.InOrder; +import org.mockito.Mockito; import java.util.Properties; -import java.util.concurrent.LinkedBlockingDeque; -import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.BlockingQueue; import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG; +import static org.apache.kafka.clients.consumer.ConsumerConfig.RETRY_BACKOFF_MS_CONFIG; import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; -public class DefaultBackgroundThreadRunnableTest { - private long refreshBackoffMs = 100; - private long expireMs = 1000; +public class DefaultBackgroundThreadTest { + private static final long REFRESH_BACK_OFF_MS = 100; private final Properties properties = new Properties(); private MockTime time; private SubscriptionState subscriptions; @@ -46,24 +53,34 @@ public class DefaultBackgroundThreadRunnableTest { private MockClient client; private LogContext context; private ConsumerNetworkClient consumerClient; + private Metrics metrics; + private BlockingQueue backgroundEventsQueue; + private BlockingQueue applicationEventsQueue; @BeforeEach + @SuppressWarnings("unchecked") public void setup() { this.time = new MockTime(); - this.subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); - this.metadata = new ConsumerMetadata(refreshBackoffMs, expireMs, false, false, - subscriptions, new LogContext(), new ClusterResourceListeners()); - this.client = new MockClient(time, metadata); + this.subscriptions = mock(SubscriptionState.class); + this.metadata = mock(ConsumerMetadata.class); this.context = new LogContext(); - this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, - 100, 1000, 100); + this.consumerClient = mock(ConsumerNetworkClient.class); + this.metrics = mock(Metrics.class); + this.applicationEventsQueue = + (BlockingQueue) mock(BlockingQueue.class); + this.backgroundEventsQueue = + (BlockingQueue) mock(BlockingQueue.class); properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + properties.put(RETRY_BACKOFF_MS_CONFIG, REFRESH_BACK_OFF_MS); } @Test public void testStartupAndTearDown() { - DefaultBackgroundThreadRunnable runnable = setupMockHandler(); + this.client = new MockClient(time, metadata); + this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, + 100, 1000, 100); + DefaultBackgroundThread runnable = setupMockHandler(); KafkaThread thread = new KafkaThread("test-thread", runnable, true); thread.start(); assertTrue(client.active()); @@ -71,15 +88,32 @@ public void testStartupAndTearDown() { assertFalse(client.active()); } - private DefaultBackgroundThreadRunnable setupMockHandler() { - DefaultBackgroundThreadRunnable runnable = new DefaultBackgroundThreadRunnable( - time, + @Test + void testNetworkAndBlockingQueuePoll() { + // ensure network poll and application queue poll will happen in a + // single iteration + DefaultBackgroundThread runnable = setupMockHandler(); + runnable.runOnce(); + + when(applicationEventsQueue.isEmpty()).thenReturn(false); + when(applicationEventsQueue.poll()).thenReturn(new NoopApplicationEvent("nothing")); + InOrder inOrder = Mockito.inOrder(applicationEventsQueue, this.consumerClient); + assertFalse(inOrder.verify(applicationEventsQueue).isEmpty()); + inOrder.verify(applicationEventsQueue).poll(); + inOrder.verify(this.consumerClient).poll(any(Timer.class)); + runnable.close(); + } + + private DefaultBackgroundThread setupMockHandler() { + return new DefaultBackgroundThread( + new MockTime(), new ConsumerConfig(properties), - new LinkedBlockingDeque<>(), - new LinkedBlockingQueue<>(), - subscriptions, - metadata, - this.consumerClient); - return runnable; + new LogContext(), + applicationEventsQueue, + backgroundEventsQueue, + this.subscriptions, + this.metadata, + this.consumerClient, + this.metrics); } } diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 734a0f7e8a194..382cd11544f46 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -25,6 +25,7 @@ import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.serialization.StringDeserializer; +import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.common.utils.Time; @@ -32,6 +33,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import java.io.Closeable; import java.util.Optional; import java.util.Properties; import java.util.concurrent.BlockingQueue; @@ -69,7 +71,9 @@ public void testBasicPollAndAddWithNoopEvent() { DefaultEventHandler handler = new DefaultEventHandler( time, new ConsumerConfig(properties), - aq, bq, + logContext, + aq, + bq, subscriptions, metadata, consumerClient); @@ -90,8 +94,10 @@ public void testBasicPollAndAddWithNoopEvent() { public void testRunOnceBackgroundThread() { BlockingQueue applicationEventQueue = new LinkedBlockingQueue<>(); BlockingQueue backgroundEventQueue = new LinkedBlockingQueue<>(); + KafkaThread backgroundThread = + new KafkaThread("", new RunOnceBackgroundThreadRunnable(applicationEventQueue, backgroundEventQueue), true); EventHandler eventHandler = new DefaultEventHandler( - new RunOnceBackgroundThreadRunnable(applicationEventQueue, backgroundEventQueue), + backgroundThread, applicationEventQueue, backgroundEventQueue); assertTrue(eventHandler.add(new NoopApplicationEvent("hello-world"))); @@ -110,7 +116,7 @@ private static ConsumerMetadata newConsumerMetadata(boolean includeInternalTopic subscriptions, new LogContext(), new ClusterResourceListeners()); } - private class RunOnceBackgroundThreadRunnable implements BackgroundThreadRunnable { + private class RunOnceBackgroundThreadRunnable implements Runnable, Closeable { private BlockingQueue applicationEventQueue; private BlockingQueue backgroundEventQueue; From 45c55f95e541e3f06f2f5156c7ee3f56ab129986 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 10 Oct 2022 20:56:13 -0700 Subject: [PATCH 35/45] Move network client construction to the handler --- .../internals/DefaultEventHandler.java | 80 +++++++++++++++++-- .../DefaultBackgroundThreadTest.java | 9 ++- .../internals/DefaultEventHandlerTest.java | 50 ------------ 3 files changed, 81 insertions(+), 58 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index cc0ae053ab09c..226b254be5c42 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -16,15 +16,20 @@ */ package org.apache.kafka.clients.consumer.internals; +import org.apache.kafka.clients.ApiVersions; import org.apache.kafka.clients.ClientUtils; +import org.apache.kafka.clients.NetworkClient; import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; +import org.apache.kafka.clients.producer.ProducerConfig; import org.apache.kafka.common.errors.InterruptException; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.metrics.Metrics; -import org.apache.kafka.common.utils.KafkaThread; +import org.apache.kafka.common.metrics.Sensor; +import org.apache.kafka.common.network.ChannelBuilder; +import org.apache.kafka.common.network.Selector; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.Time; @@ -43,9 +48,72 @@ public class DefaultEventHandler implements EventHandler { private static final String METRIC_GRP_PREFIX = "consumer"; private final BlockingQueue applicationEventQueue; private final BlockingQueue backgroundEventQueue; - private final KafkaThread backgroundThread; + private final DefaultBackgroundThread backgroundThread; + public DefaultEventHandler(Time time, + ConsumerConfig config, + LogContext logContext, + BlockingQueue applicationEventQueue, + BlockingQueue backgroundEventQueue, + SubscriptionState subscriptionState, + ApiVersions apiVersions, + Metrics metrics, + ClusterResourceListeners clusterResourceListeners, + Sensor fetcherThrottleTimeSensor) { + this.applicationEventQueue = applicationEventQueue; + this.backgroundEventQueue = backgroundEventQueue; + ConsumerMetadata metadata = bootstrapMetadata(logContext, + clusterResourceListeners, + config, subscriptionState); + ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext); + Selector selector = new Selector(config.getLong( + ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), + metrics, + time, + METRIC_GRP_PREFIX, + channelBuilder, + logContext); + NetworkClient netClient = new NetworkClient( + selector, + metadata, + config.getString(ProducerConfig.CLIENT_ID_CONFIG), + 100, // a fixed large enough value will suffice for max + // in-flight requests + config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG), + config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MAX_MS_CONFIG), + config.getInt(ConsumerConfig.SEND_BUFFER_CONFIG), + config.getInt(ConsumerConfig.RECEIVE_BUFFER_CONFIG), + config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), + config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_CONFIG), + config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG), + time, + true, + apiVersions, + fetcherThrottleTimeSensor, + logContext); + ConsumerNetworkClient networkClient = new ConsumerNetworkClient( + logContext, + netClient, + metadata, + time, + config.getInt(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), + config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), + config.getInt(ConsumerConfig.HEARTBEAT_INTERVAL_MS_CONFIG)); + this.backgroundThread = new DefaultBackgroundThread( + time, + config, + logContext, + this.applicationEventQueue, + this.backgroundEventQueue, + subscriptionState, + metadata, + networkClient, + new Metrics(time)); + } + + // VisibleForTesting + DefaultEventHandler(Time time, ConsumerConfig config, LogContext logContext, BlockingQueue applicationEventQueue, @@ -69,7 +137,7 @@ public DefaultEventHandler(Time time, } // VisibleForTesting - DefaultEventHandler(KafkaThread backgroundThread, + DefaultEventHandler(DefaultBackgroundThread backgroundThread, BlockingQueue applicationEventQueue, BlockingQueue backgroundEventQueue) { this.backgroundThread = backgroundThread; @@ -101,6 +169,9 @@ public boolean isEmpty() { @Override public boolean add(ApplicationEvent event) { try { + synchronized (backgroundThread) { + backgroundThread.notify(); + } return applicationEventQueue.add(event); } catch (IllegalStateException e) { // swallow the capacity restriction exception @@ -128,8 +199,7 @@ private ConsumerMetadata bootstrapMetadata( public void close() { try { - // this.backgroundThread.interrupt(); - // close logic + backgroundThread.close(); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index ba8e8f43b2188..4f1e48ea8eb1a 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -34,6 +34,7 @@ import java.util.Properties; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG; import static org.apache.kafka.clients.consumer.ConsumerConfig.RETRY_BACKOFF_MS_CONFIG; @@ -76,10 +77,11 @@ public void setup() { } @Test - public void testStartupAndTearDown() { + public void testStartupAndTearDown() throws InterruptedException { this.client = new MockClient(time, metadata); this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, 100, 1000, 100); + this.applicationEventsQueue = new LinkedBlockingQueue<>(); DefaultBackgroundThread runnable = setupMockHandler(); KafkaThread thread = new KafkaThread("test-thread", runnable, true); thread.start(); @@ -89,9 +91,10 @@ public void testStartupAndTearDown() { } @Test - void testNetworkAndBlockingQueuePoll() { + void testNetworkAndBlockingQueuePoll() throws InterruptedException { // ensure network poll and application queue poll will happen in a // single iteration + this.time = new MockTime(100); DefaultBackgroundThread runnable = setupMockHandler(); runnable.runOnce(); @@ -106,7 +109,7 @@ void testNetworkAndBlockingQueuePoll() { private DefaultBackgroundThread setupMockHandler() { return new DefaultBackgroundThread( - new MockTime(), + this.time, new ConsumerConfig(properties), new LogContext(), applicationEventsQueue, diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 382cd11544f46..47a3a6171236e 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -21,11 +21,9 @@ import org.apache.kafka.clients.consumer.OffsetResetStrategy; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; -import org.apache.kafka.clients.consumer.internals.events.EventHandler; import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.serialization.StringDeserializer; -import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.common.utils.Time; @@ -33,8 +31,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -import java.io.Closeable; -import java.util.Optional; import java.util.Properties; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -42,7 +38,6 @@ import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG; import static org.apache.kafka.clients.consumer.ConsumerConfig.RETRY_BACKOFF_MS_CONFIG; import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -85,28 +80,6 @@ public void testBasicPollAndAddWithNoopEvent() { } assertTrue(handler.poll().get() instanceof NoopBackgroundEvent); assertFalse(client.hasInFlightRequests()); // noop does not send network request - handler.close(); - assertFalse(client.active()); - } - - @Test - @Timeout(1) - public void testRunOnceBackgroundThread() { - BlockingQueue applicationEventQueue = new LinkedBlockingQueue<>(); - BlockingQueue backgroundEventQueue = new LinkedBlockingQueue<>(); - KafkaThread backgroundThread = - new KafkaThread("", new RunOnceBackgroundThreadRunnable(applicationEventQueue, backgroundEventQueue), true); - EventHandler eventHandler = new DefaultEventHandler( - backgroundThread, - applicationEventQueue, - backgroundEventQueue); - assertTrue(eventHandler.add(new NoopApplicationEvent("hello-world"))); - while (eventHandler.isEmpty()) { } - Optional event = eventHandler.poll(); - assertTrue(event.isPresent()); - assertTrue(event.get() instanceof NoopBackgroundEvent); - assertEquals(BackgroundEvent.EventType.NOOP, event.get().type); - assertEquals("hello-world", ((NoopBackgroundEvent) event.get()).message); } private static ConsumerMetadata newConsumerMetadata(boolean includeInternalTopics, SubscriptionState subscriptions) { @@ -115,27 +88,4 @@ private static ConsumerMetadata newConsumerMetadata(boolean includeInternalTopic return new ConsumerMetadata(refreshBackoffMs, expireMs, includeInternalTopics, false, subscriptions, new LogContext(), new ClusterResourceListeners()); } - - private class RunOnceBackgroundThreadRunnable implements Runnable, Closeable { - private BlockingQueue applicationEventQueue; - private BlockingQueue backgroundEventQueue; - - public RunOnceBackgroundThreadRunnable(BlockingQueue applicationEvents, - BlockingQueue backgroundEventQueue) { - this.applicationEventQueue = applicationEvents; - this.backgroundEventQueue = backgroundEventQueue; - } - - @Override - public void run() { - while (applicationEventQueue.isEmpty()) { } - ApplicationEvent event = applicationEventQueue.poll(); - String message = ((NoopApplicationEvent) event).message; - backgroundEventQueue.add(new NoopBackgroundEvent(message)); - } - - @Override - public void close() { - } - } } From 8f2c8709ad4f859aa1a453706bb92464bc8efb0f Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Mon, 10 Oct 2022 20:56:55 -0700 Subject: [PATCH 36/45] clean up --- .../internals/DefaultBackgroundThread.java | 43 ++++++++++++++----- .../internals/DefaultEventHandler.java | 15 ++----- .../DefaultBackgroundThreadTest.java | 8 ++-- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index 1fe8071b76bac..c1fbf385e2b23 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -27,6 +27,7 @@ import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.Time; +import org.apache.kafka.common.utils.Timer; import org.apache.kafka.common.utils.Utils; import org.slf4j.Logger; @@ -100,7 +101,7 @@ public DefaultBackgroundThread(Time time, this.config = config; setConfig(); this.inflightEvent = Optional.empty(); - // subscriptionState is initialized in the polling thread + // subscriptionState is initialized by the polling thread this.subscriptions = subscriptions; this.metadata = metadata; this.networkClient = networkClient; @@ -125,12 +126,10 @@ public void run() { log.debug("{} started", getClass()); while (running) { runOnce(); - time.sleep(retryBackoffMs); } } catch (InterruptException e) { log.error("The background thread has been interrupted"); this.exception.set(Optional.of(new RuntimeException(e))); - Thread.interrupted(); } catch (Throwable t) { log.error("The background thread failed due to unexpected error", t); @@ -147,18 +146,36 @@ public void run() { /** * Process event from a single poll */ - void runOnce() { + void runOnce() throws InterruptedException { this.inflightEvent = maybePollEvent(); - log.debug("processing applicatoin event: {}", this.inflightEvent); + log.debug("processing application event: {}", this.inflightEvent); if (this.inflightEvent.isPresent() && maybeConsumeInflightEvent(this.inflightEvent.get())) { // clear inflight event upon successful consumption this.inflightEvent = Optional.empty(); } - System.out.println(retryBackoffMs); - networkClient.poll(time.timer(this.retryBackoffMs)); + + // if there are pending events to process, poll then continue without + // blocking. + if (!applicationEventQueue.isEmpty() || inflightEvent.isPresent()) { + networkClient.poll(time.timer(0)); + return; + } + // if there are no even to process, poll and wait until timeout + Timer timer = time.timer(retryBackoffMs); + networkClient.poll(timer); + synchronized (this) { + while (waitOnCondition(timer)) this.wait(timer.remainingMs()); + } + } + + private boolean waitOnCondition(Timer timer) { + timer.update(time.milliseconds()); + return !inflightEvent.isPresent() && + applicationEventQueue.isEmpty() && + timer.notExpired(); } - public Optional maybePollEvent() { + private Optional maybePollEvent() { if (this.inflightEvent.isPresent() || this.applicationEventQueue.isEmpty()) { return this.inflightEvent; } @@ -170,7 +187,7 @@ public Optional maybePollEvent() { * @param event an {@link ApplicationEvent} * @return true when successfully consumed the event. */ - public boolean maybeConsumeInflightEvent(ApplicationEvent event) { + private boolean maybeConsumeInflightEvent(ApplicationEvent event) { log.debug("try consuming event: {}", Optional.ofNullable(event)); switch (event.type) { case NOOP: @@ -197,11 +214,15 @@ public boolean isRunning() { return this.running; } + public synchronized void wakeup() { + networkClient.wakeup(); + notify(); + } + public void close() { + this.wakeup(); this.running = false; Utils.closeQuietly(networkClient, "consumer network client"); Utils.closeQuietly(metadata, "consumer network client"); } - - } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 226b254be5c42..205f2a1c18c68 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -23,7 +23,6 @@ import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; -import org.apache.kafka.clients.producer.ProducerConfig; import org.apache.kafka.common.errors.InterruptException; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.metrics.Metrics; @@ -77,7 +76,7 @@ public DefaultEventHandler(Time time, NetworkClient netClient = new NetworkClient( selector, metadata, - config.getString(ProducerConfig.CLIENT_ID_CONFIG), + config.getString(ConsumerConfig.CLIENT_ID_CONFIG), 100, // a fixed large enough value will suffice for max // in-flight requests config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG), @@ -168,15 +167,9 @@ public boolean isEmpty() { @Override public boolean add(ApplicationEvent event) { - try { - synchronized (backgroundThread) { - backgroundThread.notify(); - } - return applicationEventQueue.add(event); - } catch (IllegalStateException e) { - // swallow the capacity restriction exception - return false; - } + boolean res = applicationEventQueue.add(event); + backgroundThread.wakeup(); + return res; } private ConsumerMetadata bootstrapMetadata( diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 4f1e48ea8eb1a..7740c7dd02a24 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -23,7 +23,6 @@ import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.serialization.StringDeserializer; -import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.common.utils.Timer; @@ -82,11 +81,10 @@ public void testStartupAndTearDown() throws InterruptedException { this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, 100, 1000, 100); this.applicationEventsQueue = new LinkedBlockingQueue<>(); - DefaultBackgroundThread runnable = setupMockHandler(); - KafkaThread thread = new KafkaThread("test-thread", runnable, true); - thread.start(); + DefaultBackgroundThread backgroundThread = setupMockHandler(); + backgroundThread.start(); assertTrue(client.active()); - runnable.close(); + backgroundThread.close(); assertFalse(client.active()); } From a343d2dc4bd4744ad7927ea5ebe5044e5e278688 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 11 Oct 2022 18:57:19 -0700 Subject: [PATCH 37/45] wakeup logic --- .../internals/DefaultBackgroundThread.java | 27 ++++++++++++------- .../internals/DefaultEventHandler.java | 2 +- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index c1fbf385e2b23..ffe95acea89b5 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -23,6 +23,7 @@ import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.KafkaException; import org.apache.kafka.common.errors.InterruptException; +import org.apache.kafka.common.errors.WakeupException; import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; @@ -125,7 +126,11 @@ public void run() { try { log.debug("{} started", getClass()); while (running) { - runOnce(); + try { + runOnce(); + } catch (WakeupException e) { + // swallow wakup + } } } catch (InterruptException e) { log.error("The background thread has been interrupted"); @@ -146,7 +151,7 @@ public void run() { /** * Process event from a single poll */ - void runOnce() throws InterruptedException { + void runOnce() { this.inflightEvent = maybePollEvent(); log.debug("processing application event: {}", this.inflightEvent); if (this.inflightEvent.isPresent() && maybeConsumeInflightEvent(this.inflightEvent.get())) { @@ -160,12 +165,15 @@ void runOnce() throws InterruptedException { networkClient.poll(time.timer(0)); return; } - // if there are no even to process, poll and wait until timeout - Timer timer = time.timer(retryBackoffMs); - networkClient.poll(timer); - synchronized (this) { - while (waitOnCondition(timer)) this.wait(timer.remainingMs()); - } + // if there are no even to process, poll until the next heartbeat. + // The networkClient will take the minimum of the requestTimeoutMs, + // nextHeartBeatMs, and nextMetadataUpdate + networkClient.poll(time.timer(timeToNextHeartbeatMs(time.milliseconds()))); + } + + private long timeToNextHeartbeatMs(long nowMs) { + // TODO: implemented when heartbeat is added to the impl + return 100; } private boolean waitOnCondition(Timer timer) { @@ -214,9 +222,8 @@ public boolean isRunning() { return this.running; } - public synchronized void wakeup() { + public void wakeup() { networkClient.wakeup(); - notify(); } public void close() { diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 205f2a1c18c68..266d90d144f1b 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -167,8 +167,8 @@ public boolean isEmpty() { @Override public boolean add(ApplicationEvent event) { - boolean res = applicationEventQueue.add(event); backgroundThread.wakeup(); + boolean res = applicationEventQueue.add(event); return res; } From 7f3424dc816f641e768977035a2172ccff46e2ad Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 11 Oct 2022 19:03:48 -0700 Subject: [PATCH 38/45] documentation --- .../internals/DefaultBackgroundThread.java | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index ffe95acea89b5..205dd3fcbfead 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -28,7 +28,6 @@ import org.apache.kafka.common.utils.KafkaThread; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.Time; -import org.apache.kafka.common.utils.Timer; import org.apache.kafka.common.utils.Utils; import org.slf4j.Logger; @@ -62,7 +61,7 @@ public class DefaultBackgroundThread extends KafkaThread { private int heartbeatIntervalMs; private boolean running; private Optional inflightEvent = Optional.empty(); - private AtomicReference> exception = + private final AtomicReference> exception = new AtomicReference<>(Optional.empty()); public DefaultBackgroundThread(ConsumerConfig config, @@ -165,9 +164,9 @@ void runOnce() { networkClient.poll(time.timer(0)); return; } - // if there are no even to process, poll until the next heartbeat. - // The networkClient will take the minimum of the requestTimeoutMs, - // nextHeartBeatMs, and nextMetadataUpdate + // if there are no events to process, poll until timeout. The timeout + // will be the minimum of the requestTimeoutMs, nextHeartBeatMs, and + // nextMetadataUpdate. See NetworkClient.poll impl. networkClient.poll(time.timer(timeToNextHeartbeatMs(time.milliseconds()))); } @@ -176,13 +175,6 @@ private long timeToNextHeartbeatMs(long nowMs) { return 100; } - private boolean waitOnCondition(Timer timer) { - timer.update(time.milliseconds()); - return !inflightEvent.isPresent() && - applicationEventQueue.isEmpty() && - timer.notExpired(); - } - private Optional maybePollEvent() { if (this.inflightEvent.isPresent() || this.applicationEventQueue.isEmpty()) { return this.inflightEvent; From d0e8f20353341af127a92e02b5a21dec09e5f7e9 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 11 Oct 2022 19:23:27 -0700 Subject: [PATCH 39/45] swallow interrupt exception --- .../consumer/internals/DefaultBackgroundThread.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index 205dd3fcbfead..38727d9cb8bf8 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -127,13 +127,10 @@ public void run() { while (running) { try { runOnce(); - } catch (WakeupException e) { - // swallow wakup + } catch (WakeupException | InterruptException e) { + // swallow } } - } catch (InterruptException e) { - log.error("The background thread has been interrupted"); - this.exception.set(Optional.of(new RuntimeException(e))); } catch (Throwable t) { log.error("The background thread failed due to unexpected error", t); From 54406644a614d9277833be4a5c85363726a16699 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 11 Oct 2022 19:39:28 -0700 Subject: [PATCH 40/45] test swallowed exception --- .../internals/DefaultBackgroundThread.java | 2 ++ .../DefaultBackgroundThreadTest.java | 31 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index 38727d9cb8bf8..96a81bd7916b9 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -128,6 +128,8 @@ public void run() { try { runOnce(); } catch (WakeupException | InterruptException e) { + log.debug("Exception thrown, background thread won't " + + "terminate", e); // swallow } } diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 7740c7dd02a24..8318bbf305bb5 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -21,6 +21,7 @@ import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; +import org.apache.kafka.common.errors.WakeupException; import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.serialization.StringDeserializer; import org.apache.kafka.common.utils.LogContext; @@ -41,7 +42,9 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; public class DefaultBackgroundThreadTest { @@ -89,7 +92,33 @@ public void testStartupAndTearDown() throws InterruptedException { } @Test - void testNetworkAndBlockingQueuePoll() throws InterruptedException { + public void testInterruption() throws InterruptedException { + this.client = new MockClient(time, metadata); + this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, + 100, 1000, 100); + this.applicationEventsQueue = new LinkedBlockingQueue<>(); + DefaultBackgroundThread backgroundThread = setupMockHandler(); + backgroundThread.start(); + assertTrue(client.active()); + backgroundThread.close(); + assertFalse(client.active()); + } + + @Test + void testBackgroundThreadSwallowedException() { + // ensure network poll and application queue poll will happen in a + // single iteration + this.time = new MockTime(100); + DefaultBackgroundThread runnable = spy(setupMockHandler()); + runnable.start(); + doThrow(WakeupException.class).when(runnable).wakeup(); + runnable.wakeup(); + assertTrue(runnable.isRunning()); + runnable.close(); + } + + @Test + void testNetworkAndBlockingQueuePoll() { // ensure network poll and application queue poll will happen in a // single iteration this.time = new MockTime(100); From 498e06b3793cc61c383e263adb113b6be902f1e5 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 12 Oct 2022 10:20:22 -0700 Subject: [PATCH 41/45] remove the test for testing --- .../internals/DefaultBackgroundThreadTest.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 8318bbf305bb5..9e7d19d9b8822 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -104,19 +104,6 @@ public void testInterruption() throws InterruptedException { assertFalse(client.active()); } - @Test - void testBackgroundThreadSwallowedException() { - // ensure network poll and application queue poll will happen in a - // single iteration - this.time = new MockTime(100); - DefaultBackgroundThread runnable = spy(setupMockHandler()); - runnable.start(); - doThrow(WakeupException.class).when(runnable).wakeup(); - runnable.wakeup(); - assertTrue(runnable.isRunning()); - runnable.close(); - } - @Test void testNetworkAndBlockingQueuePoll() { // ensure network poll and application queue poll will happen in a From f3158aad2db6af34b6215a9798d8bd0e9f2a340a Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Wed, 12 Oct 2022 10:50:15 -0700 Subject: [PATCH 42/45] wakeup exception test --- .../DefaultBackgroundThreadTest.java | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 9e7d19d9b8822..9b9cc1f9ddf42 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -40,11 +40,10 @@ import static org.apache.kafka.clients.consumer.ConsumerConfig.RETRY_BACKOFF_MS_CONFIG; import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; public class DefaultBackgroundThreadTest { @@ -53,7 +52,6 @@ public class DefaultBackgroundThreadTest { private MockTime time; private SubscriptionState subscriptions; private ConsumerMetadata metadata; - private MockClient client; private LogContext context; private ConsumerNetworkClient consumerClient; private Metrics metrics; @@ -80,7 +78,7 @@ public void setup() { @Test public void testStartupAndTearDown() throws InterruptedException { - this.client = new MockClient(time, metadata); + MockClient client = new MockClient(time, metadata); this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, 100, 1000, 100); this.applicationEventsQueue = new LinkedBlockingQueue<>(); @@ -93,7 +91,7 @@ public void testStartupAndTearDown() throws InterruptedException { @Test public void testInterruption() throws InterruptedException { - this.client = new MockClient(time, metadata); + MockClient client = new MockClient(time, metadata); this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, 100, 1000, 100); this.applicationEventsQueue = new LinkedBlockingQueue<>(); @@ -104,6 +102,22 @@ public void testInterruption() throws InterruptedException { assertFalse(client.active()); } + @Test + void testWakeup() { + this.time = new MockTime(0); + MockClient client = new MockClient(time, metadata); + this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, + 100, 1000, 100); + when(applicationEventsQueue.isEmpty()).thenReturn(true); + when(applicationEventsQueue.isEmpty()).thenReturn(true); + DefaultBackgroundThread runnable = setupMockHandler(); + client.poll(0, time.milliseconds()); + runnable.wakeup(); + + assertThrows(WakeupException.class, () -> runnable.runOnce()); + runnable.close(); + } + @Test void testNetworkAndBlockingQueuePoll() { // ensure network poll and application queue poll will happen in a From fbb59e8da1a744d47c9906fe066c7b40fffb29cf Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 18 Oct 2022 11:02:29 -0700 Subject: [PATCH 43/45] refactor based on PR comment --- .../internals/DefaultBackgroundThread.java | 24 +++++++------------ .../internals/DefaultEventHandler.java | 19 ++++----------- .../internals/PrototypeAsyncConsumer.java | 6 ++++- .../internals/events/ApplicationEvent.java | 16 +++++-------- .../internals/events/EventHandler.java | 9 ------- .../events/NoopApplicationEvent.java | 16 +++++++++++-- .../DefaultBackgroundThreadTest.java | 2 +- .../internals/DefaultEventHandlerTest.java | 3 ++- 8 files changed, 41 insertions(+), 54 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index 96a81bd7916b9..77e5133c3a644 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -22,7 +22,6 @@ import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.NoopApplicationEvent; import org.apache.kafka.common.KafkaException; -import org.apache.kafka.common.errors.InterruptException; import org.apache.kafka.common.errors.WakeupException; import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.utils.KafkaThread; @@ -127,10 +126,11 @@ public void run() { while (running) { try { runOnce(); - } catch (WakeupException | InterruptException e) { + } catch (WakeupException e) { log.debug("Exception thrown, background thread won't " + "terminate", e); - // swallow + // swallow the wakeup exception to prevent killing the + // background thread. } } } catch (Throwable t) { @@ -151,7 +151,9 @@ public void run() { */ void runOnce() { this.inflightEvent = maybePollEvent(); - log.debug("processing application event: {}", this.inflightEvent); + if (this.inflightEvent.isPresent()) { + log.debug("processing application event: {}", this.inflightEvent); + } if (this.inflightEvent.isPresent() && maybeConsumeInflightEvent(this.inflightEvent.get())) { // clear inflight event upon successful consumption this.inflightEvent = Optional.empty(); @@ -188,15 +190,7 @@ private Optional maybePollEvent() { */ private boolean maybeConsumeInflightEvent(ApplicationEvent event) { log.debug("try consuming event: {}", Optional.ofNullable(event)); - switch (event.type) { - case NOOP: - process((NoopApplicationEvent) event); - return true; - default: - inflightEvent = Optional.empty(); - log.warn("unsupported event type: {}", event.type); - return true; - } + return event.process(); } /** @@ -218,9 +212,9 @@ public void wakeup() { } public void close() { - this.wakeup(); this.running = false; + this.wakeup(); Utils.closeQuietly(networkClient, "consumer network client"); - Utils.closeQuietly(metadata, "consumer network client"); + Utils.closeQuietly(metadata, "consumer metadata client"); } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index 266d90d144f1b..ab2a5eb53e262 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -23,7 +23,6 @@ import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; -import org.apache.kafka.common.errors.InterruptException; import org.apache.kafka.common.internals.ClusterResourceListeners; import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.metrics.Sensor; @@ -33,11 +32,9 @@ import org.apache.kafka.common.utils.Time; import java.net.InetSocketAddress; -import java.time.Duration; import java.util.List; import java.util.Optional; import java.util.concurrent.BlockingQueue; -import java.util.concurrent.TimeUnit; /** * An {@code EventHandler} that uses a single background thread to consume {@code ApplicationEvent} and produce @@ -150,16 +147,6 @@ public Optional poll() { return Optional.ofNullable(backgroundEventQueue.poll()); } - @Override - public Optional poll(Duration timeout) { - try { - return Optional.ofNullable(backgroundEventQueue.poll(timeout.toMillis(), - TimeUnit.MILLISECONDS)); - } catch (InterruptedException e) { - throw new InterruptException(e); - } - } - @Override public boolean isEmpty() { return backgroundEventQueue.isEmpty(); @@ -168,10 +155,12 @@ public boolean isEmpty() { @Override public boolean add(ApplicationEvent event) { backgroundThread.wakeup(); - boolean res = applicationEventQueue.add(event); - return res; + return applicationEventQueue.add(event); } + // bootstrap a metadata object with the bootstrap server IP address, + // which will be used once for the subsequent metadata refresh once the + // background thread has started up. private ConsumerMetadata bootstrapMetadata( LogContext logContext, ClusterResourceListeners clusterResourceListeners, diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java index 8a1ccfc2a5ed1..2872241f8018e 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java @@ -124,7 +124,11 @@ private class CommitApplicationEvent extends ApplicationEvent { CompletableFuture commitFuture = new CompletableFuture<>(); public CommitApplicationEvent() { - super(EventType.NOOP); + } + + @Override + public boolean process() { + return true; } } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java index 25cbe678bc3ce..683681a19fb14 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java @@ -20,14 +20,10 @@ * This is the abstract definition of the events created by the KafkaConsumer API */ abstract public class ApplicationEvent { - public final EventType type; - - public ApplicationEvent(EventType type) { - this.type = type; - } - - public enum EventType { - COMMIT, - NOOP, - } + /** + * process the application event. Return true upon succesful execution, + * false otherwise. + * @return true if the event was successfully executed; false otherwise. + */ + public abstract boolean process(); } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java index cc648a483f788..c07cc33cf7353 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java @@ -16,7 +16,6 @@ */ package org.apache.kafka.clients.consumer.internals.events; -import java.time.Duration; import java.util.Optional; /** @@ -30,14 +29,6 @@ public interface EventHandler { */ Optional poll(); - /** - * Retrieves and removes a {@link BackgroundEvent}, waiting up to the {@code - * timeout} for an event to become available; - * @return an Optional of {@link BackgroundEvent} if the value is present - * . Otherwise, an empty Optional. - */ - Optional poll(Duration timeout); - /** * Check whether there are pending {@code BackgroundEvent} await to be consumed. * @return true if there are no pending event diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java index 1a56be8fa5300..6a8fab5ea964d 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java @@ -16,15 +16,27 @@ */ package org.apache.kafka.clients.consumer.internals.events; +import org.apache.kafka.clients.consumer.internals.NoopBackgroundEvent; + +import java.util.concurrent.BlockingQueue; + /** * The event is NoOp. This is intentionally left here for demonstration purpose. */ public class NoopApplicationEvent extends ApplicationEvent { public final String message; + private final BlockingQueue backgroundEventQueue; - public NoopApplicationEvent(String message) { - super(EventType.NOOP); + public NoopApplicationEvent( + BlockingQueue backgroundEventQueue, + String message) { this.message = message; + this.backgroundEventQueue = backgroundEventQueue; + } + + @Override + public boolean process() { + return backgroundEventQueue.add(new NoopBackgroundEvent(message)); } @Override diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 9b9cc1f9ddf42..357814a25bdc4 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -127,7 +127,7 @@ void testNetworkAndBlockingQueuePoll() { runnable.runOnce(); when(applicationEventsQueue.isEmpty()).thenReturn(false); - when(applicationEventsQueue.poll()).thenReturn(new NoopApplicationEvent("nothing")); + when(applicationEventsQueue.poll()).thenReturn(new NoopApplicationEvent(backgroundEventsQueue, "nothing")); InOrder inOrder = Mockito.inOrder(applicationEventsQueue, this.consumerClient); assertFalse(inOrder.verify(applicationEventsQueue).isEmpty()); inOrder.verify(applicationEventsQueue).poll(); diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 47a3a6171236e..5b7758c333248 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -74,7 +74,8 @@ public void testBasicPollAndAddWithNoopEvent() { consumerClient); assertTrue(client.active()); assertTrue(handler.isEmpty()); - handler.add(new NoopApplicationEvent("testBasicPollAndAddWithNoopEvent")); + handler.add(new NoopApplicationEvent(bq, + "testBasicPollAndAddWithNoopEvent")); while (handler.isEmpty()) { time.sleep(100); } From bec9391d6b559ff77dfc43474d2c0919887fbd55 Mon Sep 17 00:00:00 2001 From: Philip Nee Date: Tue, 18 Oct 2022 11:03:59 -0700 Subject: [PATCH 44/45] debug message --- .../clients/consumer/internals/DefaultBackgroundThread.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index 77e5133c3a644..dfa4947113c97 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -122,7 +122,7 @@ private void setConfig() { @Override public void run() { try { - log.debug("{} started", getClass()); + log.debug("Background thread started"); while (running) { try { runOnce(); From f10d53011a90d8ab2e09aaabcbf8aafb66f46790 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Wed, 19 Oct 2022 18:23:43 -0500 Subject: [PATCH 45/45] apply AK code conventions --- .../internals/DefaultBackgroundThread.java | 90 +++++---- .../internals/DefaultEventHandler.java | 191 +++++++++--------- .../internals/NoopBackgroundEvent.java | 2 +- .../internals/PrototypeAsyncConsumer.java | 20 +- .../events/NoopApplicationEvent.java | 6 +- .../DefaultBackgroundThreadTest.java | 79 +++++--- .../internals/DefaultEventHandlerTest.java | 76 ++++--- 7 files changed, 266 insertions(+), 198 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index dfa4947113c97..546e0e8e76fb1 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -30,6 +30,7 @@ import org.apache.kafka.common.utils.Utils; import org.slf4j.Logger; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.BlockingQueue; import java.util.concurrent.atomic.AtomicReference; @@ -38,13 +39,13 @@ * Background thread runnable that consumes {@code ApplicationEvent} and * produces {@code BackgroundEvent}. It uses an event loop to consume and * produce events, and poll the network client to handle network IO. - * + *

* It holds a reference to the {@link SubscriptionState}, which is * initialized by the polling thread. */ public class DefaultBackgroundThread extends KafkaThread { private static final String BACKGROUND_THREAD_NAME = - "consumer_background_thread"; + "consumer_background_thread"; private final Time time; private final Logger log; private final BlockingQueue applicationEventQueue; @@ -61,36 +62,38 @@ public class DefaultBackgroundThread extends KafkaThread { private boolean running; private Optional inflightEvent = Optional.empty(); private final AtomicReference> exception = - new AtomicReference<>(Optional.empty()); - - public DefaultBackgroundThread(ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue, - SubscriptionState subscriptions, - ConsumerMetadata metadata, - ConsumerNetworkClient networkClient, - Metrics metrics) { - this(Time.SYSTEM, - config, - logContext, - applicationEventQueue, - backgroundEventQueue, - subscriptions, - metadata, - networkClient, - metrics); + new AtomicReference<>(Optional.empty()); + + public DefaultBackgroundThread(final ConsumerConfig config, + final LogContext logContext, + final BlockingQueue applicationEventQueue, + final BlockingQueue backgroundEventQueue, + final SubscriptionState subscriptions, + final ConsumerMetadata metadata, + final ConsumerNetworkClient networkClient, + final Metrics metrics) { + this( + Time.SYSTEM, + config, + logContext, + applicationEventQueue, + backgroundEventQueue, + subscriptions, + metadata, + networkClient, + metrics + ); } - public DefaultBackgroundThread(Time time, - ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue, - SubscriptionState subscriptions, - ConsumerMetadata metadata, - ConsumerNetworkClient networkClient, - Metrics metrics) { + public DefaultBackgroundThread(final Time time, + final ConsumerConfig config, + final LogContext logContext, + final BlockingQueue applicationEventQueue, + final BlockingQueue backgroundEventQueue, + final SubscriptionState subscriptions, + final ConsumerMetadata metadata, + final ConsumerNetworkClient networkClient, + final Metrics metrics) { super(BACKGROUND_THREAD_NAME, true); try { this.time = time; @@ -106,7 +109,7 @@ public DefaultBackgroundThread(Time time, this.networkClient = networkClient; this.metrics = metrics; this.running = true; - } catch (Exception e) { + } catch (final Exception e) { // now propagate the exception close(); throw new KafkaException("Failed to construct background processor", e); @@ -126,16 +129,20 @@ public void run() { while (running) { try { runOnce(); - } catch (WakeupException e) { - log.debug("Exception thrown, background thread won't " + - "terminate", e); + } catch (final WakeupException e) { + log.debug( + "Exception thrown, background thread won't terminate", + e + ); // swallow the wakeup exception to prevent killing the // background thread. } } - } catch (Throwable t) { - log.error("The background thread failed due to unexpected error", - t); + } catch (final Throwable t) { + log.error( + "The background thread failed due to unexpected error", + t + ); if (t instanceof RuntimeException) this.exception.set(Optional.of((RuntimeException) t)); else @@ -171,7 +178,7 @@ void runOnce() { networkClient.poll(time.timer(timeToNextHeartbeatMs(time.milliseconds()))); } - private long timeToNextHeartbeatMs(long nowMs) { + private long timeToNextHeartbeatMs(final long nowMs) { // TODO: implemented when heartbeat is added to the impl return 100; } @@ -185,11 +192,13 @@ private Optional maybePollEvent() { /** * ApplicationEvent are consumed here. + * * @param event an {@link ApplicationEvent} * @return true when successfully consumed the event. */ - private boolean maybeConsumeInflightEvent(ApplicationEvent event) { + private boolean maybeConsumeInflightEvent(final ApplicationEvent event) { log.debug("try consuming event: {}", Optional.ofNullable(event)); + Objects.requireNonNull(event); return event.process(); } @@ -197,9 +206,10 @@ private boolean maybeConsumeInflightEvent(ApplicationEvent event) { * Processes {@link NoopApplicationEvent} and equeue a * {@link NoopBackgroundEvent}. This is intentionally left here for * demonstration purpose. + * * @param event a {@link NoopApplicationEvent} */ - private void process(NoopApplicationEvent event) { + private void process(final NoopApplicationEvent event) { backgroundEventQueue.add(new NoopBackgroundEvent(event.message)); } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java index ab2a5eb53e262..3a5a43abf7d73 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandler.java @@ -47,95 +47,104 @@ public class DefaultEventHandler implements EventHandler { private final DefaultBackgroundThread backgroundThread; - public DefaultEventHandler(Time time, - ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue, - SubscriptionState subscriptionState, - ApiVersions apiVersions, - Metrics metrics, - ClusterResourceListeners clusterResourceListeners, - Sensor fetcherThrottleTimeSensor) { + public DefaultEventHandler(final Time time, + final ConsumerConfig config, + final LogContext logContext, + final BlockingQueue applicationEventQueue, + final BlockingQueue backgroundEventQueue, + final SubscriptionState subscriptionState, + final ApiVersions apiVersions, + final Metrics metrics, + final ClusterResourceListeners clusterResourceListeners, + final Sensor fetcherThrottleTimeSensor) { this.applicationEventQueue = applicationEventQueue; this.backgroundEventQueue = backgroundEventQueue; - ConsumerMetadata metadata = bootstrapMetadata(logContext, - clusterResourceListeners, - config, subscriptionState); - ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext); - Selector selector = new Selector(config.getLong( - ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), - metrics, - time, - METRIC_GRP_PREFIX, - channelBuilder, - logContext); - NetworkClient netClient = new NetworkClient( - selector, - metadata, - config.getString(ConsumerConfig.CLIENT_ID_CONFIG), - 100, // a fixed large enough value will suffice for max - // in-flight requests - config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG), - config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MAX_MS_CONFIG), - config.getInt(ConsumerConfig.SEND_BUFFER_CONFIG), - config.getInt(ConsumerConfig.RECEIVE_BUFFER_CONFIG), - config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), - config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_CONFIG), - config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG), - time, - true, - apiVersions, - fetcherThrottleTimeSensor, - logContext); - ConsumerNetworkClient networkClient = new ConsumerNetworkClient( - logContext, - netClient, - metadata, - time, - config.getInt(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), - config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), - config.getInt(ConsumerConfig.HEARTBEAT_INTERVAL_MS_CONFIG)); + final ConsumerMetadata metadata = bootstrapMetadata( + logContext, + clusterResourceListeners, + config, + subscriptionState + ); + final ChannelBuilder channelBuilder = ClientUtils.createChannelBuilder(config, time, logContext); + final Selector selector = new Selector( + config.getLong( + ConsumerConfig.CONNECTIONS_MAX_IDLE_MS_CONFIG), + metrics, + time, + METRIC_GRP_PREFIX, + channelBuilder, + logContext + ); + final NetworkClient netClient = new NetworkClient( + selector, + metadata, + config.getString(ConsumerConfig.CLIENT_ID_CONFIG), + 100, // a fixed large enough value will suffice for max + // in-flight requests + config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MS_CONFIG), + config.getLong(ConsumerConfig.RECONNECT_BACKOFF_MAX_MS_CONFIG), + config.getInt(ConsumerConfig.SEND_BUFFER_CONFIG), + config.getInt(ConsumerConfig.RECEIVE_BUFFER_CONFIG), + config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), + config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_CONFIG), + config.getLong(ConsumerConfig.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG), + time, + true, + apiVersions, + fetcherThrottleTimeSensor, + logContext + ); + final ConsumerNetworkClient networkClient = new ConsumerNetworkClient( + logContext, + netClient, + metadata, + time, + config.getInt(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), + config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG), + config.getInt(ConsumerConfig.HEARTBEAT_INTERVAL_MS_CONFIG) + ); this.backgroundThread = new DefaultBackgroundThread( - time, - config, - logContext, - this.applicationEventQueue, - this.backgroundEventQueue, - subscriptionState, - metadata, - networkClient, - new Metrics(time)); + time, + config, + logContext, + this.applicationEventQueue, + this.backgroundEventQueue, + subscriptionState, + metadata, + networkClient, + new Metrics(time) + ); } // VisibleForTesting - DefaultEventHandler(Time time, - ConsumerConfig config, - LogContext logContext, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue, - SubscriptionState subscriptionState, - ConsumerMetadata metadata, - ConsumerNetworkClient networkClient) { + DefaultEventHandler(final Time time, + final ConsumerConfig config, + final LogContext logContext, + final BlockingQueue applicationEventQueue, + final BlockingQueue backgroundEventQueue, + final SubscriptionState subscriptionState, + final ConsumerMetadata metadata, + final ConsumerNetworkClient networkClient) { this.applicationEventQueue = applicationEventQueue; this.backgroundEventQueue = backgroundEventQueue; this.backgroundThread = new DefaultBackgroundThread( - time, - config, - logContext, - this.applicationEventQueue, - this.backgroundEventQueue, - subscriptionState, - metadata, - networkClient, - new Metrics(time)); + time, + config, + logContext, + this.applicationEventQueue, + this.backgroundEventQueue, + subscriptionState, + metadata, + networkClient, + new Metrics(time) + ); backgroundThread.start(); } // VisibleForTesting - DefaultEventHandler(DefaultBackgroundThread backgroundThread, - BlockingQueue applicationEventQueue, - BlockingQueue backgroundEventQueue) { + DefaultEventHandler(final DefaultBackgroundThread backgroundThread, + final BlockingQueue applicationEventQueue, + final BlockingQueue backgroundEventQueue) { this.backgroundThread = backgroundThread; this.applicationEventQueue = applicationEventQueue; this.backgroundEventQueue = backgroundEventQueue; @@ -153,7 +162,7 @@ public boolean isEmpty() { } @Override - public boolean add(ApplicationEvent event) { + public boolean add(final ApplicationEvent event) { backgroundThread.wakeup(); return applicationEventQueue.add(event); } @@ -162,19 +171,19 @@ public boolean add(ApplicationEvent event) { // which will be used once for the subsequent metadata refresh once the // background thread has started up. private ConsumerMetadata bootstrapMetadata( - LogContext logContext, - ClusterResourceListeners clusterResourceListeners, - ConsumerConfig config, - SubscriptionState subscriptions) { - ConsumerMetadata metadata = new ConsumerMetadata( - config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), - config.getLong(ConsumerConfig.METADATA_MAX_AGE_CONFIG), - !config.getBoolean(ConsumerConfig.EXCLUDE_INTERNAL_TOPICS_CONFIG), - config.getBoolean(ConsumerConfig.ALLOW_AUTO_CREATE_TOPICS_CONFIG), - subscriptions, - logContext, clusterResourceListeners); - List addresses = ClientUtils.parseAndValidateAddresses( - config.getList(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG), config.getString(ConsumerConfig.CLIENT_DNS_LOOKUP_CONFIG)); + final LogContext logContext, + final ClusterResourceListeners clusterResourceListeners, + final ConsumerConfig config, + final SubscriptionState subscriptions) { + final ConsumerMetadata metadata = new ConsumerMetadata( + config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), + config.getLong(ConsumerConfig.METADATA_MAX_AGE_CONFIG), + !config.getBoolean(ConsumerConfig.EXCLUDE_INTERNAL_TOPICS_CONFIG), + config.getBoolean(ConsumerConfig.ALLOW_AUTO_CREATE_TOPICS_CONFIG), + subscriptions, + logContext, clusterResourceListeners); + final List addresses = ClientUtils.parseAndValidateAddresses( + config.getList(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG), config.getString(ConsumerConfig.CLIENT_DNS_LOOKUP_CONFIG)); metadata.bootstrap(addresses); return metadata; } @@ -182,7 +191,7 @@ private ConsumerMetadata bootstrapMetadata( public void close() { try { backgroundThread.close(); - } catch (Exception e) { + } catch (final Exception e) { throw new RuntimeException(e); } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java index bbedbcace7652..db6ed2f7a82dd 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoopBackgroundEvent.java @@ -24,7 +24,7 @@ public class NoopBackgroundEvent extends BackgroundEvent { public final String message; - public NoopBackgroundEvent(String message) { + public NoopBackgroundEvent(final String message) { super(EventType.NOOP); this.message = message; } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java index 2872241f8018e..060fc3b50cdc1 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java @@ -49,6 +49,7 @@ public PrototypeAsyncConsumer(final Time time, final EventHandler eventHandler) * another type of event, process it. * 2. Send fetches if needed. * If the timeout expires, return an empty ConsumerRecord. + * * @param timeout timeout of the poll loop * @return ConsumerRecord. It can be empty if time timeout expires. */ @@ -57,7 +58,7 @@ public ConsumerRecords poll(final Duration timeout) { try { do { if (!eventHandler.isEmpty()) { - Optional backgroundEvent = eventHandler.poll(); + final Optional backgroundEvent = eventHandler.poll(); // processEvent() may process 3 types of event: // 1. Errors // 2. Callback Invocation @@ -76,7 +77,7 @@ public ConsumerRecords poll(final Duration timeout) { } // We will wait for retryBackoffMs } while (time.timer(timeout).notExpired()); - } catch (Exception e) { + } catch (final Exception e) { throw new RuntimeException(e); } @@ -84,7 +85,9 @@ public ConsumerRecords poll(final Duration timeout) { } abstract void processEvent(BackgroundEvent backgroundEvent, Duration timeout); + abstract ConsumerRecords processFetchResults(Fetch fetch); + abstract Fetch collectFetches(); /** @@ -92,25 +95,26 @@ public ConsumerRecords poll(final Duration timeout) { */ @Override public void commitAsync() { - ApplicationEvent commitEvent = new CommitApplicationEvent(); + final ApplicationEvent commitEvent = new CommitApplicationEvent(); eventHandler.add(commitEvent); } /** * This method sends a commit event to the EventHandler and waits for the event to finish. + * * @param timeout max wait time for the blocking operation. */ @Override - public void commitSync(Duration timeout) { - CommitApplicationEvent commitEvent = new CommitApplicationEvent(); + public void commitSync(final Duration timeout) { + final CommitApplicationEvent commitEvent = new CommitApplicationEvent(); eventHandler.add(commitEvent); - CompletableFuture commitFuture = commitEvent.commitFuture; + final CompletableFuture commitFuture = commitEvent.commitFuture; try { commitFuture.get(timeout.toMillis(), TimeUnit.MILLISECONDS); - } catch (TimeoutException e) { + } catch (final TimeoutException e) { throw new org.apache.kafka.common.errors.TimeoutException("timeout"); - } catch (Exception e) { + } catch (final Exception e) { // handle exception here throw new RuntimeException(e); } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java index 6a8fab5ea964d..0fe9dccb1039a 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/NoopApplicationEvent.java @@ -27,9 +27,9 @@ public class NoopApplicationEvent extends ApplicationEvent { public final String message; private final BlockingQueue backgroundEventQueue; - public NoopApplicationEvent( - BlockingQueue backgroundEventQueue, - String message) { + public NoopApplicationEvent(final BlockingQueue backgroundEventQueue, + final String message) { + this.message = message; this.backgroundEventQueue = backgroundEventQueue; } diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 357814a25bdc4..f159fad6bc521 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -67,10 +67,8 @@ public void setup() { this.context = new LogContext(); this.consumerClient = mock(ConsumerNetworkClient.class); this.metrics = mock(Metrics.class); - this.applicationEventsQueue = - (BlockingQueue) mock(BlockingQueue.class); - this.backgroundEventsQueue = - (BlockingQueue) mock(BlockingQueue.class); + this.applicationEventsQueue = (BlockingQueue) mock(BlockingQueue.class); + this.backgroundEventsQueue = (BlockingQueue) mock(BlockingQueue.class); properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(RETRY_BACKOFF_MS_CONFIG, REFRESH_BACK_OFF_MS); @@ -78,11 +76,18 @@ public void setup() { @Test public void testStartupAndTearDown() throws InterruptedException { - MockClient client = new MockClient(time, metadata); - this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, - 100, 1000, 100); + final MockClient client = new MockClient(time, metadata); + this.consumerClient = new ConsumerNetworkClient( + context, + client, + metadata, + time, + 100, + 1000, + 100 + ); this.applicationEventsQueue = new LinkedBlockingQueue<>(); - DefaultBackgroundThread backgroundThread = setupMockHandler(); + final DefaultBackgroundThread backgroundThread = setupMockHandler(); backgroundThread.start(); assertTrue(client.active()); backgroundThread.close(); @@ -91,11 +96,18 @@ public void testStartupAndTearDown() throws InterruptedException { @Test public void testInterruption() throws InterruptedException { - MockClient client = new MockClient(time, metadata); - this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, - 100, 1000, 100); + final MockClient client = new MockClient(time, metadata); + this.consumerClient = new ConsumerNetworkClient( + context, + client, + metadata, + time, + 100, + 1000, + 100 + ); this.applicationEventsQueue = new LinkedBlockingQueue<>(); - DefaultBackgroundThread backgroundThread = setupMockHandler(); + final DefaultBackgroundThread backgroundThread = setupMockHandler(); backgroundThread.start(); assertTrue(client.active()); backgroundThread.close(); @@ -105,16 +117,23 @@ public void testInterruption() throws InterruptedException { @Test void testWakeup() { this.time = new MockTime(0); - MockClient client = new MockClient(time, metadata); - this.consumerClient = new ConsumerNetworkClient(context, client, metadata, time, - 100, 1000, 100); + final MockClient client = new MockClient(time, metadata); + this.consumerClient = new ConsumerNetworkClient( + context, + client, + metadata, + time, + 100, + 1000, + 100 + ); when(applicationEventsQueue.isEmpty()).thenReturn(true); when(applicationEventsQueue.isEmpty()).thenReturn(true); - DefaultBackgroundThread runnable = setupMockHandler(); + final DefaultBackgroundThread runnable = setupMockHandler(); client.poll(0, time.milliseconds()); runnable.wakeup(); - assertThrows(WakeupException.class, () -> runnable.runOnce()); + assertThrows(WakeupException.class, runnable::runOnce); runnable.close(); } @@ -123,12 +142,13 @@ void testNetworkAndBlockingQueuePoll() { // ensure network poll and application queue poll will happen in a // single iteration this.time = new MockTime(100); - DefaultBackgroundThread runnable = setupMockHandler(); + final DefaultBackgroundThread runnable = setupMockHandler(); runnable.runOnce(); when(applicationEventsQueue.isEmpty()).thenReturn(false); - when(applicationEventsQueue.poll()).thenReturn(new NoopApplicationEvent(backgroundEventsQueue, "nothing")); - InOrder inOrder = Mockito.inOrder(applicationEventsQueue, this.consumerClient); + when(applicationEventsQueue.poll()) + .thenReturn(new NoopApplicationEvent(backgroundEventsQueue, "nothing")); + final InOrder inOrder = Mockito.inOrder(applicationEventsQueue, this.consumerClient); assertFalse(inOrder.verify(applicationEventsQueue).isEmpty()); inOrder.verify(applicationEventsQueue).poll(); inOrder.verify(this.consumerClient).poll(any(Timer.class)); @@ -137,14 +157,15 @@ void testNetworkAndBlockingQueuePoll() { private DefaultBackgroundThread setupMockHandler() { return new DefaultBackgroundThread( - this.time, - new ConsumerConfig(properties), - new LogContext(), - applicationEventsQueue, - backgroundEventsQueue, - this.subscriptions, - this.metadata, - this.consumerClient, - this.metrics); + this.time, + new ConsumerConfig(properties), + new LogContext(), + applicationEventsQueue, + backgroundEventsQueue, + this.subscriptions, + this.metadata, + this.consumerClient, + this.metrics + ); } } diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java index 5b7758c333248..0da35e834945e 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultEventHandlerTest.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import java.util.Optional; import java.util.Properties; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -54,39 +55,62 @@ public void setup() { @Test @Timeout(1) public void testBasicPollAndAddWithNoopEvent() { - Time time = new MockTime(1); - LogContext logContext = new LogContext(); - SubscriptionState subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); - ConsumerMetadata metadata = newConsumerMetadata(false, subscriptions); - MockClient client = new MockClient(time, metadata); - ConsumerNetworkClient consumerClient = new ConsumerNetworkClient(logContext, client, metadata, time, - 100, 1000, 100); - BlockingQueue aq = new LinkedBlockingQueue<>(); - BlockingQueue bq = new LinkedBlockingQueue<>(); - DefaultEventHandler handler = new DefaultEventHandler( - time, - new ConsumerConfig(properties), - logContext, - aq, - bq, - subscriptions, - metadata, - consumerClient); + final Time time = new MockTime(1); + final LogContext logContext = new LogContext(); + final SubscriptionState subscriptions = new SubscriptionState(new LogContext(), OffsetResetStrategy.NONE); + final ConsumerMetadata metadata = newConsumerMetadata(false, subscriptions); + final MockClient client = new MockClient(time, metadata); + final ConsumerNetworkClient consumerClient = new ConsumerNetworkClient( + logContext, + client, + metadata, + time, + 100, + 1000, + 100 + ); + final BlockingQueue aq = new LinkedBlockingQueue<>(); + final BlockingQueue bq = new LinkedBlockingQueue<>(); + final DefaultEventHandler handler = new DefaultEventHandler( + time, + new ConsumerConfig(properties), + logContext, + aq, + bq, + subscriptions, + metadata, + consumerClient + ); assertTrue(client.active()); assertTrue(handler.isEmpty()); - handler.add(new NoopApplicationEvent(bq, - "testBasicPollAndAddWithNoopEvent")); + handler.add( + new NoopApplicationEvent( + bq, + "testBasicPollAndAddWithNoopEvent" + ) + ); while (handler.isEmpty()) { time.sleep(100); } - assertTrue(handler.poll().get() instanceof NoopBackgroundEvent); + final Optional poll = handler.poll(); + assertTrue(poll.isPresent()); + assertTrue(poll.get() instanceof NoopBackgroundEvent); + assertFalse(client.hasInFlightRequests()); // noop does not send network request } - private static ConsumerMetadata newConsumerMetadata(boolean includeInternalTopics, SubscriptionState subscriptions) { - long refreshBackoffMs = 50; - long expireMs = 50000; - return new ConsumerMetadata(refreshBackoffMs, expireMs, includeInternalTopics, false, - subscriptions, new LogContext(), new ClusterResourceListeners()); + private static ConsumerMetadata newConsumerMetadata(final boolean includeInternalTopics, + final SubscriptionState subscriptions) { + final long refreshBackoffMs = 50; + final long expireMs = 50000; + return new ConsumerMetadata( + refreshBackoffMs, + expireMs, + includeInternalTopics, + false, + subscriptions, + new LogContext(), + new ClusterResourceListeners() + ); } }