Support live reload for SmallRye Reactive Messaging #15986
Conversation
Live reload check is performed with each incoming or outgoing message, as processed by SmallRye Reactive Messaging connectors.
| @@ -228,7 +228,7 @@ public void afterEach(ExtensionContext extensionContext) throws Exception { | |||
| FileUtil.deleteDirectory(deploymentDir); | |||
| } | |||
| } | |||
| rootLogger.removeHandler(inMemoryLogHandler); | |||
| inMemoryLogHandler.clearRecords(); | |||
There was a problem hiding this comment.
Not exactly sure why we used to remove the in-memory log handler in afterEach, because that means capturing the log could only possibly have worked in the first test to be executed within one QuarkusDevModeTest instance. All the other remaining tests, should there be any, would see the captured log from the previous test, which seems obviously wrong.
Clearing the remembered content of the in-memory log handler, but leaving it attached to the root logger, seems like what should have been done since the beginning, but I'm not sure if there are any consequences I can't see.
There was a problem hiding this comment.
Yeah, the existing behavior seems wrong.... FWIW, QuarkusProdModeTest already does what @Ladicek did here
|
CI passed in my fork, and I don't see anything to be added here. Marking as ready. |
|
Just a thought, with the http connector (Quarkus-http), how is it going to behave? Double reload? @michalszynkiewicz ? |
|
Hmm good point, I didn't check that :-) But I guess that the live reload facility would ignore a 2nd request if 1st is still in progress? Well, I'd better look at it. |
|
I don't know either :) my guess would be that first reload would happen and then on the next "check for reload" would see no new changes, but that's just a guess |
| @DevModeSupportConnectorFactory | ||
| @Priority(Interceptor.Priority.PLATFORM_BEFORE + 10) | ||
| public class DevModeSupportConnectorFactoryInterceptor { | ||
| private static Supplier<CompletableFuture<Boolean>> onMessage; |
There was a problem hiding this comment.
This should have been volatile
There was a problem hiding this comment.
Ah that is true. I copied this pattern from other dev mode handlers, without too much thinking. I'll submit a PR.
| @Override | ||
| public void onNext(Message<?> o) { | ||
| subscriber.onNext(o); | ||
| onMessage.get().join(); |
There was a problem hiding this comment.
Won't this block the IO thread? Also won't the restart happen after the message has been delivered, rather than before, so the message is handled by the old code?
There was a problem hiding this comment.
Hum, right - this method is going to be called on this I/O thread. Why is it require to join actually?
In theory, the connectors are passing the reactive streams TCK meaning that the onNext / onError / onComplete methods cannot be called concurrently (but could be called by different threads).
Also, we may want to reverse the onNext and onMessage call to handle the message with the new code.
There was a problem hiding this comment.
I actually did try something like onMessage.get().whenComplete((value, error) -> subscriber.onNext(o)), but that only works well with instrumentation-based reload. With full restart, you've got a new stream running, and no way to push the message there.
Note that this is @Outgoing -- so there's no user code handling the message (unless we had some kind of "message interceptors" per smallrye/smallrye-reactive-messaging#1141 :-) ). It's only the connector code at this point, submitting the message to a broker (or whatever is on the other side).
I trigger restart after submitting the message, so that the message isn't lost. Note that even if I triggered restart before submitting the message, it would still be processed by old code with full restart, because full restart takes a few seconds during which old code still runs. (At least that's what I observed.) And I wait for restart to finish so that no other message may come in and be lost. I realize blocking is a crude way to do it, but I figured that wouldn't be such a big deal in dev mode.
I'm open to other ideas of course.
I toyed with the idea of using the request protocol to handle this, but I really don't feel like I could do it correctly :-)
Live reload check is performed with each incoming or outgoing
message, as processed by SmallRye Reactive Messaging connectors.
Resolves #3217