refactor: update producer code to work with latest packages #373

Joel-hanson · 2025-08-20T05:44:11Z

This pull request refactors the demo application's deployment and Kafka integration for improved reliability and modern best practices. The main changes include switching to a KRaft-based Kafka setup in Docker, updating configuration to use container networking, and refactoring the PeriodicProducer logic for better resource management and error handling.

Deployment and Kafka setup:

Migrated the Docker Compose configuration to use a single KRaft-based Kafka broker (apache/kafka:latest) instead of separate Zookeeper and Kafka containers, including health checks and updated environment variables for KRaft operation. The demo app now waits for Kafka health before starting. (docker-compose.yml, docker-compose.ymlL1-R34)
Added a multi-stage Dockerfile to build the Java application with Maven and run it in a slim OpenJDK container for production use. (Dockerfile, DockerfileR1-R22)
Updated kafka.properties to use the container name (kafka:9092) for bootstrap servers, ensuring correct connectivity when running in Docker. (kafka.properties, kafka.propertiesL1-R1)

Codebase improvements:

Refactored PeriodicProducer.java to use Vert.x timers instead of TimeoutStream, improved Kafka producer setup with explicit serializers, streamlined config handling, and added proper cleanup in the stop() method to close the producer gracefully. (src/main/java/kafka/vertx/demo/PeriodicProducer.java, [1] [2]
Changed the WebSocket server to bind to 0.0.0.0 for compatibility with Docker container networking. (src/main/java/kafka/vertx/demo/WebSocketServer.java, src/main/java/kafka/vertx/demo/WebSocketServer.javaL97-R96)

Checklist

Automated tests exist
Documentation exists link
Local unit tests performed
Sufficient logging/trace
Desired commit message set as PR title and commit description set above

aswinayyolath · 2025-08-20T05:49:01Z

please can you fix Git Lint issue?

Copilot

Pull Request Overview

This PR refactors the demo application to use modern Kafka deployment with KRaft mode instead of Zookeeper, improves Docker containerization, and enhances the producer code with better resource management and error handling.

Migrated from Zookeeper-based Kafka to KRaft-based single broker setup for simplified deployment
Added multi-stage Dockerfile for containerized application builds and deployment
Refactored PeriodicProducer to use Vert.x timers instead of TimeoutStream with proper resource cleanup

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
docker-compose.yml	Replaced Zookeeper+Kafka with single KRaft broker and added containerized demo app
Dockerfile	Added multi-stage build for Maven compilation and slim runtime container
kafka.properties	Updated bootstrap servers to use container networking (broker:9092)
PeriodicProducer.java	Refactored to use Vert.x timers, improved config handling, and added proper cleanup
WebSocketServer.java	Updated server binding and simplified Kafka consumer subscription logic

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

kafka.properties

src/main/java/kafka/vertx/demo/PeriodicProducer.java

src/main/java/kafka/vertx/demo/WebSocketServer.java

Dockerfile

aswinayyolath · 2025-08-20T10:09:04Z

docker-compose.yml

-      "bin/zookeeper-server-start.sh config/zookeeper.properties"
-    ]
+  broker:
+    image: apache/kafka:latest


Is this the best option...it can break builds when latest changes. is pinning a version the best way forward?

Yes that make sense. Will change that

aswinayyolath · 2025-08-20T10:15:28Z

src/main/java/kafka/vertx/demo/PeriodicProducer.java

+  private static final long PRODUCE_INTERVAL_MS = Duration.ofSeconds(2).toMillis();
+
+  private KafkaProducer<String, String> kafkaProducer;
+  private long timerId = -1;


can't we use something like

private static final long TIMER_NOT_SET = -1L; private long timerId = TIMER_NOT_SET;

Right now timerId is initialized with TIMER_NOT_SET, but later in STOP_ACTION it’s reset using the raw -1. This duplicates the Placeholder value . For clarity and maintainability, isn't it better to use the TIMER_NOT_SET constant consistently instead of mixing it with a literal.

aswinayyolath · 2025-08-20T10:23:07Z

src/main/java/kafka/vertx/demo/PeriodicProducer.java

+  public void stop() {
+    if (kafkaProducer != null) {
+      kafkaProducer.close()
+        .onComplete(ar -> logger.info("KafkaProducer closed: {}", ar.succeeded() ? "success" : ar.cause()));


will ar.cause()) be null is success ??

logger.info("KafkaProducer closed: {}", ar.succeeded() ? "success" : "failure", ar.cause());

No, that won't get called

aswinayyolath · 2025-08-20T10:28:32Z

src/main/java/kafka/vertx/demo/PeriodicProducer.java

+      case WebSocketServer.STOP_ACTION:
+        if (timerId != TIMER_NOT_SET) {
+          vertx.cancelTimer(timerId);
+          timerId = -1;


timerId = TIMER_NOT_SET;

aswinayyolath · 2025-08-20T10:31:44Z

src/main/java/kafka/vertx/demo/PeriodicProducer.java

+
+  private KafkaProducer<String, String> kafkaProducer;
+  private long timerId = -1;
+  private long TIMER_NOT_SET = -1;
  private String customMessage;


This will only get val when START_ACTION is received... So I tjink, If for some reason produceKafkaRecord() is ever called before the first START_ACTION, customMessage will be null.. so in that case producer could send null messages to Kafka is that ok? can't we initialize customMessage like

private String customMessage = "Hello World";

aswinayyolath · 2025-08-20T11:42:11Z

src/main/java/kafka/vertx/demo/WebSocketServer.java

@@ -140,11 +138,7 @@ private void handleProduceSocket(ServerWebSocket webSocket) {

  private void handleConsumeSocket(ServerWebSocket webSocket) {
    KafkaConsumer<String, JsonObject> kafkaConsumer = KafkaConsumer.create(vertx, kafkaConfig);
-
-    kafkaConsumer.exceptionHandler(err -> logger.error("Kafka error", err));


Should we add it back for better error visibility?

yes, you are right

* Added new Dockerfile and docker compose file to test locally Resolves: #0 Signed-off-by: Joel Hanson <joelhanson025@gmail.com>

Joel-hanson requested a review from Copilot August 20, 2025 05:44

This comment was marked as outdated.

Sign in to view

Joel-hanson force-pushed the refactor-producer branch from 39bb4be to fa3af21 Compare August 20, 2025 05:47

Joel-hanson force-pushed the refactor-producer branch 4 times, most recently from 34cf782 to 10e3b26 Compare August 20, 2025 05:57

Joel-hanson requested a review from Copilot August 20, 2025 05:57

This comment was marked as outdated.

Sign in to view

Joel-hanson force-pushed the refactor-producer branch 2 times, most recently from bdd4f76 to 53109ff Compare August 20, 2025 08:56

Joel-hanson requested a review from Copilot August 20, 2025 08:57

Copilot AI reviewed Aug 20, 2025

View reviewed changes

kafka.properties Show resolved Hide resolved

src/main/java/kafka/vertx/demo/PeriodicProducer.java Outdated Show resolved Hide resolved

src/main/java/kafka/vertx/demo/WebSocketServer.java Show resolved Hide resolved

src/main/java/kafka/vertx/demo/WebSocketServer.java Show resolved Hide resolved

aswinayyolath approved these changes Aug 20, 2025

View reviewed changes

aswinayyolath reviewed Aug 20, 2025

View reviewed changes

Joel-hanson force-pushed the refactor-producer branch from 53109ff to 3008c8f Compare August 21, 2025 04:02

refactor: update producer code to work with latest packages

02d3c11

* Added new Dockerfile and docker compose file to test locally Resolves: #0 Signed-off-by: Joel Hanson <joelhanson025@gmail.com>

Joel-hanson force-pushed the refactor-producer branch from 3008c8f to 02d3c11 Compare August 21, 2025 04:44

Joel-hanson requested a review from aswinayyolath August 21, 2025 04:44

aswinayyolath approved these changes Aug 21, 2025

View reviewed changes

Joel-hanson merged commit ec03822 into ibm-messaging:master Aug 21, 2025
2 checks passed

Joel-hanson deleted the refactor-producer branch August 21, 2025 04:50

refactor: update producer code to work with latest packages #373

refactor: update producer code to work with latest packages #373

Uh oh!

Conversation

Joel-hanson commented Aug 20, 2025

Checklist

Uh oh!

This comment was marked as outdated.

Uh oh!

aswinayyolath commented Aug 20, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!