-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add discovery feature for broker to auto load services #8107
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
Add discovery feature for broker to auto load services #8107
Conversation
| * To make a class auto-loaded, what we need to do is to add the below dependency into project: | ||
| * <p> | ||
| * <pre> | ||
| * <dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I use
{@code .... } , our check-style plugin will complain and fail the build.Here I was forced to use HTML encode
|
|
||
| @Service | ||
| @Singleton | ||
| public class BrokerTestAutoLoadedService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is not visible to BrokerAdminApiApplication.It is not registered by BrokerAdminApiApplication either.
But this service can be created and Injected into EchoWithAutoDiscovery endpoint above
| </dependency> | ||
| <dependency> | ||
| <groupId>org.glassfish.hk2</groupId> | ||
| <artifactId>hk2-metadata-generator</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module is needed in any project that wish its classes to be auto-discovered.
When building the project, this module will create META-INF/hk2-locator/default file in JAR, and write the class name into that filer properly.
Later the ServiceAutoDiscoveryFeature class will automatically load the class.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8107 +/- ##
============================================
+ Coverage 71.35% 71.43% +0.07%
Complexity 4303 4303
============================================
Files 1617 1620 +3
Lines 83874 83891 +17
Branches 12537 12538 +1
============================================
+ Hits 59851 59928 +77
+ Misses 19943 19880 -63
- Partials 4080 4083 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
High level, this is good. |
Good question, I was thinking about enabling the same feature for controller/server/minion too, so did not call it |
|
@xiangfu0 the build has succeeded. |
Description
Added an optional "Auto Discovery" feature in PinotBroker so services tagged with
@Servicetag can be auto-loaded.This will make Pinot Broker's Jersey behave very similar like Spring Framework in terms of service discovery.
pinot.broker.service.auto.discoveryto enable this behaviorSteps needed to make a class auto-created in PinotBroker:
hk2-metadata-generatormodule as a dependency in your JAR file@org.jvnet.hk2.annotations.Servicetagpinot.broker.service.auto.discovery=truein your Broker's property configUpgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
backward-incompat, and complete the section below on Release Notes)No
Does this PR fix a zero-downtime upgrade introduced earlier?
backward-incompat, and complete the section below on Release Notes)No
Does this PR otherwise need attention when creating release notes? Things to consider:
pinot.broker.service.auto.discoveryso Jersey services can be created automatically and then later injected to Endpoints.release-notesand complete the section on Release Notes)Release Notes
Release note: Added Service Auto-discovery feature into PinotBroker so classes annotated with
@Serviceand built withhk2-metadata-generatormodule can be automatically loaded.Documentation
Will add later