You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "sclee01 (via GitHub)" <gi...@apache.org> on 2026/10/22 04:50:39 UTC

[PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

sclee01 opened a new pull request, #5847:
URL: https://github.com/apache/polaris/pull/5847

   ### Problem
   
   `EventBusConfigurer` registers a default codec for `PolarisEvent` on every `StartupEvent`, but never unregisters it.
   In Quarkus dev mode the Vert.x EventBus survives a live reload, so the second registration throws the exception (`Already a codec registered with name polaris-events-local`) and the application cannot restart until the dev server is stopped.
   
     ### Change
   
   Unregister the codec in a `@PreDestroy` method, following the existing `@PreDestroy` cleanup pattern.
   The `EventBus` is now injected as a field so that it is available at destroy time.
   
     ### Testing
     - Added `EventBusConfigurerTest`, which registers, unregisters and registers again. It fails without the fix and passes with it.
   - Manually verified in dev mode: after adding a blank line to a Java file and sending a request, the app restarts and serves the request. The log shows unregister -> register on each reload.
   - `./gradlew :polaris-runtime-service:check` passes locally.
   
     ## Checklist
     - [x] ๐Ÿ›ก๏ธ  Don't disclose security issues! (contact security@apache.org)
     - [x] ๐Ÿ”— Clearly explained why the changes are needed, or linked related issues: Fixes #5844
     - [x] ๐Ÿงช Added/updated tests with good coverage, or manually tested (and explained how)
     - [ ] ๐Ÿ’ก Added comments for complex logic
     - [ ] ๐Ÿงพ Updated `CHANGELOG.md` (if needed)
     - [ ] ๐Ÿ“š Updated documentation in `site/content/in-dev/unreleased` (if needed)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

Posted by "sclee01 (via GitHub)" <gi...@apache.org>.
sclee01 commented on code in PR #5847:
URL: https://github.com/apache/polaris/pull/5847#discussion_r4355341466


##########
runtime/service/src/main/java/org/apache/polaris/service/events/EventBusConfigurer.java:
##########
@@ -22,14 +22,23 @@
 import io.quarkus.runtime.StartupEvent;
 import io.quarkus.vertx.LocalEventBusCodec;
 import io.vertx.core.eventbus.EventBus;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
 
 @ApplicationScoped
 public class EventBusConfigurer {
   private static final String LOCAL_CODEC_NAME = "polaris-events-local";
 
-  void configureEventBus(@Observes StartupEvent ev, EventBus eventBus) {
+  @Inject EventBus eventBus;
+
+  void configureEventBus(@Observes StartupEvent ev) {
     eventBus.registerDefaultCodec(PolarisEvent.class, new LocalEventBusCodec<>(LOCAL_CODEC_NAME));
   }
+
+  @PreDestroy

Review Comment:
   @orca-review-bot 
   Thanks. This only matters in the last moment of shutdown: a background task publishing after the codec is removed would not get its event delivered. Since event delivery is best-effort, I think that's acceptable, but I can catch and log it in the dispatcher if you prefer.
   
   I tried unregistering right before registering: it doesn't fix the reload. After a reload `PolarisEvent.class` comes from a new class loader, so the old entry isn't found and the name still clashes. @PreDestroy removes it with the old class, so I'd keep it.



##########
runtime/service/src/main/java/org/apache/polaris/service/events/EventBusConfigurer.java:
##########
@@ -22,14 +22,23 @@
 import io.quarkus.runtime.StartupEvent;
 import io.quarkus.vertx.LocalEventBusCodec;
 import io.vertx.core.eventbus.EventBus;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
 
 @ApplicationScoped
 public class EventBusConfigurer {
   private static final String LOCAL_CODEC_NAME = "polaris-events-local";
 
-  void configureEventBus(@Observes StartupEvent ev, EventBus eventBus) {
+  @Inject EventBus eventBus;
+
+  void configureEventBus(@Observes StartupEvent ev) {
     eventBus.registerDefaultCodec(PolarisEvent.class, new LocalEventBusCodec<>(LOCAL_CODEC_NAME));
   }
+
+  @PreDestroy

Review Comment:
   @orca-review-bot 
   Thanks. This only matters in the last moment of shutdown: a background task publishing after the codec is removed would not get its event delivered. Since event delivery is best-effort, I think that's acceptable, but I can catch and log it in the dispatcher if you prefer.
   
   I tried unregistering right before registering: it doesn't fix the reload. After a reload `PolarisEvent.class` comes from a new class loader, so the old entry isn't found and the name still clashes. `@PreDestroy `removes it with the old class, so I'd keep it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #5847:
URL: https://github.com/apache/polaris/pull/5847#discussion_r4350482845


##########
runtime/service/src/main/java/org/apache/polaris/service/events/EventBusConfigurer.java:
##########
@@ -22,14 +22,23 @@
 import io.quarkus.runtime.StartupEvent;
 import io.quarkus.vertx.LocalEventBusCodec;
 import io.vertx.core.eventbus.EventBus;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
 
 @ApplicationScoped
 public class EventBusConfigurer {
   private static final String LOCAL_CODEC_NAME = "polaris-events-local";
 
-  void configureEventBus(@Observes StartupEvent ev, EventBus eventBus) {
+  @Inject EventBus eventBus;
+
+  void configureEventBus(@Observes StartupEvent ev) {

Review Comment:
   Maybe rename to `registerEventCodec` ? Then `unregisterCodec` => `unregisterEventCodec`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

Posted by "orca-review-bot (via GitHub)" <gi...@apache.org>.
orca-review-bot commented on code in PR #5847:
URL: https://github.com/apache/polaris/pull/5847#discussion_r4351245198


##########
runtime/service/src/main/java/org/apache/polaris/service/events/EventBusConfigurer.java:
##########
@@ -22,14 +22,23 @@
 import io.quarkus.runtime.StartupEvent;
 import io.quarkus.vertx.LocalEventBusCodec;
 import io.vertx.core.eventbus.EventBus;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
 
 @ApplicationScoped
 public class EventBusConfigurer {
   private static final String LOCAL_CODEC_NAME = "polaris-events-local";
 
-  void configureEventBus(@Observes StartupEvent ev, EventBus eventBus) {
+  @Inject EventBus eventBus;
+
+  void configureEventBus(@Observes StartupEvent ev) {
     eventBus.registerDefaultCodec(PolarisEvent.class, new LocalEventBusCodec<>(LOCAL_CODEC_NAME));
   }
+
+  @PreDestroy

Review Comment:
   ArC doesn't promise any order for destroying beans at shutdown. So this codec could be removed while something can still publish a `PolarisEvent`, for example a background task finishing up. If that happens, the publish in `PolarisServiceBusEventDispatcher` fails with a missing-codec error. This is probably rare and only happens at shutdown. Is that OK, or would you rather unregister right before registering in `configureEventBus` so the codec stays until Vert.x itself goes down?
   
   <sub>๐Ÿค– AI-generated review comment โ€” agents make mistakes, so please verify before acting on it.</sub>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

Posted by "sclee01 (via GitHub)" <gi...@apache.org>.
sclee01 commented on code in PR #5847:
URL: https://github.com/apache/polaris/pull/5847#discussion_r4355326548


##########
runtime/service/src/main/java/org/apache/polaris/service/events/EventBusConfigurer.java:
##########
@@ -22,14 +22,23 @@
 import io.quarkus.runtime.StartupEvent;
 import io.quarkus.vertx.LocalEventBusCodec;
 import io.vertx.core.eventbus.EventBus;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
 
 @ApplicationScoped
 public class EventBusConfigurer {
   private static final String LOCAL_CODEC_NAME = "polaris-events-local";
 
-  void configureEventBus(@Observes StartupEvent ev, EventBus eventBus) {
+  @Inject EventBus eventBus;
+
+  void configureEventBus(@Observes StartupEvent ev) {

Review Comment:
   @adutra 
   Good idea, thanks. Renamed to registerEventCodec, unregisterEventCodec and the test accordingly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra merged PR #5847:
URL: https://github.com/apache/polaris/pull/5847


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

Posted by "sclee01 (via GitHub)" <gi...@apache.org>.
sclee01 commented on code in PR #5847:
URL: https://github.com/apache/polaris/pull/5847#discussion_r4355341466


##########
runtime/service/src/main/java/org/apache/polaris/service/events/EventBusConfigurer.java:
##########
@@ -22,14 +22,23 @@
 import io.quarkus.runtime.StartupEvent;
 import io.quarkus.vertx.LocalEventBusCodec;
 import io.vertx.core.eventbus.EventBus;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
 
 @ApplicationScoped
 public class EventBusConfigurer {
   private static final String LOCAL_CODEC_NAME = "polaris-events-local";
 
-  void configureEventBus(@Observes StartupEvent ev, EventBus eventBus) {
+  @Inject EventBus eventBus;
+
+  void configureEventBus(@Observes StartupEvent ev) {
     eventBus.registerDefaultCodec(PolarisEvent.class, new LocalEventBusCodec<>(LOCAL_CODEC_NAME));
   }
+
+  @PreDestroy

Review Comment:
   @orca-review-bot 
   Thanks. This only matters in the last moment of shutdown: a background task publishing after the codec is removed would not get its event delivered. Since event delivery is best-effort, I think that's acceptable, but I can catch and log it in the dispatcher if you prefer.
   
   I tried unregistering right before registering: it doesn't fix the reload. After a reload `PolarisEvent.class` comes from a new class loader, so the old entry isn't found and the name still clashes. `@PreDestroy` removes it with the old class, so I'd keep it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix dev mode live reload failing with duplicate event bus codec [polaris]

Posted by "sclee01 (via GitHub)" <gi...@apache.org>.
sclee01 commented on code in PR #5847:
URL: https://github.com/apache/polaris/pull/5847#discussion_r4355341466


##########
runtime/service/src/main/java/org/apache/polaris/service/events/EventBusConfigurer.java:
##########
@@ -22,14 +22,23 @@
 import io.quarkus.runtime.StartupEvent;
 import io.quarkus.vertx.LocalEventBusCodec;
 import io.vertx.core.eventbus.EventBus;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
+import jakarta.inject.Inject;
 
 @ApplicationScoped
 public class EventBusConfigurer {
   private static final String LOCAL_CODEC_NAME = "polaris-events-local";
 
-  void configureEventBus(@Observes StartupEvent ev, EventBus eventBus) {
+  @Inject EventBus eventBus;
+
+  void configureEventBus(@Observes StartupEvent ev) {
     eventBus.registerDefaultCodec(PolarisEvent.class, new LocalEventBusCodec<>(LOCAL_CODEC_NAME));
   }
+
+  @PreDestroy

Review Comment:
   @orca-review-bot 
   Thanks. This only matters in the last moment of shutdown: a background task publishing
   after the codec is removed would not get its event delivered. Since event delivery is
   best-effort, I think that's acceptable, but I can catch and log it in the dispatcher if
   you prefer.
   
   I tried unregistering right before registering: it doesn't fix the reload. After a reload
   PolarisEvent.class comes from a new class loader, so the old entry isn't found and the
   name still clashes. @PreDestroy removes it with the old class, so I'd keep it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org