Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger hot reload for reactive messaging #3217

Closed
mswiderski opened this issue Jul 14, 2019 · 12 comments · Fixed by #15986
Closed

Trigger hot reload for reactive messaging #3217

mswiderski opened this issue Jul 14, 2019 · 12 comments · Fixed by #15986

Comments

@mswiderski
Copy link
Contributor

Description
Trigger hot reload of the application when new Kafka message is received, would be great to have it supported not only for Kafka but all transports supported by reactive messaging

Relates to discussion thread https://groups.google.com/d/msgid/quarkus-dev/5F5FFC95-3C70-4756-8191-13847D63D540%40gmail.com?utm_medium=email&utm_source=footer

@mswiderski mswiderski added the kind/enhancement New feature or request label Jul 14, 2019
@stale
Copy link

stale bot commented Nov 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you!
We are doing this automatically to ensure out-of-date issues does not stay around indefinitely.
If you believe this issue is still relevant please put a comment on it on why and if it truly needs to stay request or add 'pinned' label.

@stale stale bot added the stale label Nov 13, 2019
@mswiderski
Copy link
Contributor Author

I'd say this is still very relevant to have a complete store on developer workflow when using reactive messaging.

@stale stale bot removed the stale label Nov 13, 2019
@gsmet gsmet added the pinned Issue will never be marked as stale label Nov 13, 2019
@gsmet
Copy link
Member

gsmet commented Nov 13, 2019

See also #2709 .

@gsmet gsmet changed the title Trigger hot reload on new Kafka message Trigger hot reload for reactive messaging Nov 18, 2019
@emmanuelbernard
Copy link
Member

@cescoffier is that still something to be done? Or is it done? Just for cleanup purposes

@emmanuelbernard emmanuelbernard added the triage/consider-closing Bugs that are considered to be closed because too old. Using the label to do a mark and sweep proces label Apr 1, 2020
@gsmet
Copy link
Member

gsmet commented Apr 2, 2020

No, it's not done and we had several people asking for it.

@gsmet gsmet removed the triage/consider-closing Bugs that are considered to be closed because too old. Using the label to do a mark and sweep proces label Apr 2, 2020
@cescoffier
Copy link
Member

cescoffier commented Mar 17, 2021

@Ladicek that could be an interesting one, if you are interested.

The idea is to check for "reload" when we receive a message. There are multiple ways to do it.

  1. Protocol specific

Basically, for Kafka, we can reproduce what we do for Kafka Streams. We introduce an interceptor and trigger the hot reload. To avoid too many verifications, we introduce a grace period.

I'm not sure we would be able to do the same for AMQP (there is no interceptors support in the client)

  1. Protocol agnostic

This is more complicated. We can imagine introducing an interceptor in the connectors to do this.
Connectors are beans, so we can "intercept" them in dev mode, and introduce the reload logic when the connector emits a message. Something like:

PublisherBuilder pb = connector.getPublisherBuilder(Config config);
return pb.peek(m -> triggerReloadIfNeeded());

We just need to be sure to not lose the inflight message (that can be the tricky part).

@cescoffier cescoffier removed pinned Issue will never be marked as stale triage/gsmet-christmas-list labels Mar 17, 2021
@Ladicek
Copy link
Contributor

Ladicek commented Mar 17, 2021

I guess we want this for all protocols, not just Kafka (but if Kafka is easier, perhaps we can just start there). And, as mentioned above, not just when receiving a message, but perhaps also when emitting a message. Interesting one indeed :-) I'll finish some unfinished tasks and take a look at this. Nice opportunity to get deeper into Quarkus internals (hope I won't regret it :-) ).

@Ladicek
Copy link
Contributor

Ladicek commented Mar 18, 2021

I have a little bit of a prototype here: https://github.com/Ladicek/quarkus/commits/reactive-messaging-live-reload It seems to function well for instrumentation-based reload, with Kafka. Need to verify with other brokers and full restart, and then of course write some tests (gonna be interesting).

@Ladicek
Copy link
Contributor

Ladicek commented Mar 23, 2021

I've pushed something that behaves, I believe, reasonably well -- both with instrumentation and full restart (tested with Kafka and AMQP on Artemis). I'm pretty sure there are some small windows for message loss, but frankly, I don't think that's a big deal in dev mode. I still need to figure out how to write tests.

@cescoffier
Copy link
Member

Awesome, did you open a PR?

@cescoffier
Copy link
Member

Data loss should be mitigated by missing ack - so basically, it should be still in the queue / topic

@Ladicek
Copy link
Contributor

Ladicek commented Mar 23, 2021

Need to create some tests, PR will obviously follow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants