You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/02/11 01:01:22 UTC
[GitHub] [pinot] dongxiaoman opened a new pull request #8193: enable service discovery in controller too
dongxiaoman opened a new pull request #8193:
URL: https://github.com/apache/pinot/pull/8193
## Description
<!-- Add a description of your PR here.
A good description should include pointers to an issue or design document, etc.
-->
Enable controller service auto-discovery in Jersey framework.
This is following https://github.com/apache/pinot/pull/8107 to enable the service auto-creation in controller too.
It is found out that we may need to hook in new services in Pinot Controllers too.
The context can be found in previous PR, I will copy/paste the same text here for reference:
1. Main idea is behind [mkyong's article in detail in here](https://mkyong.com/webservices/jax-rs/jersey-and-hk2-dependency-injection-auto-scanning/)
2. Added a property boolean named `pinot.controller.service.auto.discovery` to enable this behavior
Steps needed to make a class auto-created in PinotController:
1. Add `hk2-metadata-generator` module as a dependency in your JAR file
2. Tag your class with `@org.jvnet.hk2.annotations.Service` tag
3. Set `pinot.controller.service.auto.discovery=true` in your Controller's property config
4. Make sure your JAR file is loaded in class path(of coz)
5. (Optional) inject your class instance to other places
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
- New configuration options: `pinot.controller.service.auto.discovery` so Jersey services can be created automatically and then later injected to Endpoints.
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] dongxiaoman commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
dongxiaoman commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1039359232
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a change in pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on a change in pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#discussion_r805314378
##########
File path: pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ControllerServiceDiscoveryIntegrationTest.java
##########
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.integration.tests;
+
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.apache.pinot.util.TestUtils;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+/**
+ * Integration test that starts one broker with auto-discovered echo service and test it
+ */
+public class ControllerServiceDiscoveryIntegrationTest extends BaseClusterIntegrationTestSet {
+ private static final String TENANT_NAME = "TestTenant";
+
+ @Override
+ protected String getBrokerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ protected String getServerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ public Map<String, Object> getDefaultControllerConfiguration() {
+ Map<String, Object> retVal = super.getDefaultControllerConfiguration();
+ retVal.put(CommonConstants.Controller.CONTROLLER_SERVICE_AUTO_DISCOVERY, true);
+ return retVal;
+ }
+
+ @Override
+ protected PinotConfiguration getDefaultBrokerConfiguration() {
+ PinotConfiguration config = new PinotConfiguration();
+ config.setProperty(CommonConstants.Broker.BROKER_SERVICE_AUTO_DISCOVERY, true);
+ return config;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir);
+
+ // Start the Pinot cluster
+ startZk();
+ startController();
+ startBrokers(1);
+ startServers(1);
+
+ }
Review comment:
https://github.com/apache/pinot/pull/8197
You triggered my OCD...
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1039393067
Great, no concerns my end then.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] dongxiaoman edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
dongxiaoman edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1039359232
> @richardstartin the discovery is only tried on the class written in `META-INF/hk2-locator/default` file. In my example the file contains `[org.apache.pinot.broker.write.api.context.WriteApiContextProvider]S`. The file is written automatically for the class tagged with `@service` tag.Based on code reading, it reads from that file and try to load it. There is no "scan blindly" in place, so it should be a fixed small cost.
Strictly speaking, this feature is not "AutoDiscovery", it is something "Loading-In-RunTime-with-Specified-Class-Name-in Resource"
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a change in pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on a change in pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#discussion_r804961034
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/api/ServiceAutoDiscoveryFeature.java
##########
@@ -68,8 +69,8 @@
* pinot-integration-tests/src/main/java/org/apache/pinot/broker/integration/tests/BrokerTestAutoLoadedService.java
* </code>
*/
-public class BrokerServiceAutoDiscoveryFeature implements Feature {
- private static final Logger LOGGER = LoggerFactory.getLogger(BrokerServiceAutoDiscoveryFeature.class);
+public class ServiceAutoDiscoveryFeature implements Feature {
Review comment:
What will happen if we have PinotServiceManager deploying controller/broker/server in the same JVM and all of them trying to load this `ServiceAutoDiscoveryFeature`?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9638065) into [master](https://codecov.io/gh/apache/pinot/commit/22b17c3391be5d239bee879d5102123b7d385edc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22b17c3) will **decrease** coverage by `1.19%`.
> The diff coverage is `33.33%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
============================================
- Coverage 71.36% 70.16% -1.20%
- Complexity 4308 4311 +3
============================================
Files 1623 1624 +1
Lines 84365 84369 +4
Branches 12657 12658 +1
============================================
- Hits 60208 59199 -1009
- Misses 20032 21077 +1045
+ Partials 4125 4093 -32
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.55% <60.00%> (-0.03%)` | :arrow_down: |
| unittests1 | `67.90% <0.00%> (+0.01%)` | :arrow_up: |
| unittests2 | `14.27% <0.00%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `89.09% <0.00%> (-3.37%)` | :arrow_down: |
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `23.63% <ø> (ø)` | |
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `93.02% <100.00%> (ø)` | |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `81.81% <100.00%> (ø)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-94.74%)` | :arrow_down: |
| ... and [106 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [22b17c3...9638065](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b28d6dd) into [master](https://codecov.io/gh/apache/pinot/commit/58f74ce3a9decd02cefeaef79691d9940f509e7f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (58f74ce) will **increase** coverage by `0.04%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
============================================
+ Coverage 71.36% 71.40% +0.04%
Complexity 4308 4308
============================================
Files 1623 1624 +1
Lines 84356 84360 +4
Branches 12654 12655 +1
============================================
+ Hits 60197 60235 +38
+ Misses 20023 20001 -22
+ Partials 4136 4124 -12
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.90% <80.00%> (+0.03%)` | :arrow_up: |
| integration2 | `27.58% <60.00%> (-0.11%)` | :arrow_down: |
| unittests1 | `67.88% <0.00%> (-0.01%)` | :arrow_down: |
| unittests2 | `14.22% <0.00%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `23.63% <ø> (ø)` | |
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `93.02% <100.00%> (ø)` | |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `92.72% <100.00%> (+0.27%)` | :arrow_up: |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `81.81% <100.00%> (ø)` | |
| [...pinot/common/utils/fetcher/HttpSegmentFetcher.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZmV0Y2hlci9IdHRwU2VnbWVudEZldGNoZXIuamF2YQ==) | `61.53% <0.00%> (-10.26%)` | :arrow_down: |
| [...a/org/apache/pinot/common/utils/ServiceStatus.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU2VydmljZVN0YXR1cy5qYXZh) | `60.00% <0.00%> (-7.15%)` | :arrow_down: |
| [.../pinot/server/starter/helix/BaseServerStarter.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9CYXNlU2VydmVyU3RhcnRlci5qYXZh) | `57.98% <0.00%> (-1.97%)` | :arrow_down: |
| ... and [26 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [58f74ce...b28d6dd](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#discussion_r805149480
##########
File path: pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ControllerServiceDiscoveryIntegrationTest.java
##########
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.integration.tests;
+
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.apache.pinot.util.TestUtils;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+/**
+ * Integration test that starts one broker with auto-discovered echo service and test it
+ */
+public class ControllerServiceDiscoveryIntegrationTest extends BaseClusterIntegrationTestSet {
+ private static final String TENANT_NAME = "TestTenant";
+
+ @Override
+ protected String getBrokerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ protected String getServerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ public Map<String, Object> getDefaultControllerConfiguration() {
+ Map<String, Object> retVal = super.getDefaultControllerConfiguration();
+ retVal.put(CommonConstants.Controller.CONTROLLER_SERVICE_AUTO_DISCOVERY, true);
+ return retVal;
+ }
+
+ @Override
+ protected PinotConfiguration getDefaultBrokerConfiguration() {
+ PinotConfiguration config = new PinotConfiguration();
+ config.setProperty(CommonConstants.Broker.BROKER_SERVICE_AUTO_DISCOVERY, true);
+ return config;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir);
+
+ // Start the Pinot cluster
+ startZk();
+ startController();
+ startBrokers(1);
+ startServers(1);
+
+ }
Review comment:
I’m surprised the linter passed, there should be a blank line here. We will need to check if checkstyle can catch this.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] dongxiaoman commented on a change in pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
dongxiaoman commented on a change in pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#discussion_r805059710
##########
File path: pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ControllerServiceDiscoveryIntegrationTest.java
##########
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.integration.tests;
+
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.apache.pinot.util.TestUtils;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+/**
+ * Integration test that starts one broker with auto-discovered echo service and test it
+ */
+public class ControllerServiceDiscoveryIntegrationTest extends BaseClusterIntegrationTestSet {
+ private static final String TENANT_NAME = "TestTenant";
+
+ @Override
+ protected String getBrokerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ protected String getServerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ public Map<String, Object> getDefaultControllerConfiguration() {
+ Map<String, Object> retVal = super.getDefaultControllerConfiguration();
+ retVal.put(CommonConstants.Controller.CONTROLLER_SERVICE_AUTO_DISCOVERY, true);
+ return retVal;
+ }
+
+ @Override
+ protected PinotConfiguration getDefaultBrokerConfiguration() {
+ PinotConfiguration config = new PinotConfiguration();
+ config.setProperty(CommonConstants.Broker.BROKER_SERVICE_AUTO_DISCOVERY, true);
+ return config;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir);
+
+ // Start the Pinot cluster
+ startZk();
+ startController();
+ startBrokers(1);
+ startServers(1);
+
+ }
+ @AfterClass
+ public void tearDown()
+ throws Exception {
+
+ // Brokers and servers has been stopped
+ stopBroker();
+ stopController();
+ stopZk();
+ FileUtils.deleteDirectory(_tempDir);
+ }
+
+ @Test
+ public void testControllerExtraEndpointsAutoLoaded()
+ throws Exception {
+ String response = sendGetRequest(_controllerBaseApiUrl + "/test/echo/doge");
+ Assert.assertEquals(response, "doge");
+ response = sendGetRequest(_brokerBaseApiUrl + "/test/echo/doge");
Review comment:
@xiangfu0 the test here will fail if we do initialization only once
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] dongxiaoman commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
dongxiaoman commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1039359232
> @richardstartin the discovery is only tried on the class written in `META-INF/hk2-locator/default` file. In my example the file contains `[org.apache.pinot.broker.write.api.context.WriteApiContextProvider]S`. Based on code reading, it reads from that file and try to load it. There is no "scan blindly" in place, so it should be a fixed small cost.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin merged pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin merged pull request #8193:
URL: https://github.com/apache/pinot/pull/8193
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1038188674
> > High level question: does this affect startup time?
>
> I think this more comes from each customized Feature implementation?
Is there a discovery cost?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] dongxiaoman edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
dongxiaoman edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1039359232
> @richardstartin the discovery is only tried on the class written in `META-INF/hk2-locator/default` file. In my example the file contains `[org.apache.pinot.broker.write.api.context.WriteApiContextProvider]S`. The file is written automatically for the class tagged with `@service` tag.Based on code reading, it reads from that file and try to load it. There is no "scan blindly" in place, so it should be a fixed small cost.
Strictly speaking, this feature is not "AutoDiscovery", it is something "Loading-In-RunTime-with-Specified-Class-Name-in Resource"
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9638065) into [master](https://codecov.io/gh/apache/pinot/commit/22b17c3391be5d239bee879d5102123b7d385edc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22b17c3) will **decrease** coverage by `57.09%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
=============================================
- Coverage 71.36% 14.27% -57.10%
+ Complexity 4308 81 -4227
=============================================
Files 1623 1579 -44
Lines 84365 82488 -1877
Branches 12657 12454 -203
=============================================
- Hits 60208 11775 -48433
- Misses 20032 69832 +49800
+ Partials 4125 881 -3244
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.27% <0.00%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `89.09% <0.00%> (-3.37%)` | :arrow_down: |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (-23.64%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1298 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [22b17c3...9638065](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] dongxiaoman commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
dongxiaoman commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1040534891
@xiangfu0 Gentle ping: build and test passed
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1039393067
Great, no concerns my end then.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a change in pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on a change in pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#discussion_r805314378
##########
File path: pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ControllerServiceDiscoveryIntegrationTest.java
##########
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.integration.tests;
+
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.apache.pinot.util.TestUtils;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+/**
+ * Integration test that starts one broker with auto-discovered echo service and test it
+ */
+public class ControllerServiceDiscoveryIntegrationTest extends BaseClusterIntegrationTestSet {
+ private static final String TENANT_NAME = "TestTenant";
+
+ @Override
+ protected String getBrokerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ protected String getServerTenant() {
+ return TENANT_NAME;
+ }
+
+ @Override
+ public Map<String, Object> getDefaultControllerConfiguration() {
+ Map<String, Object> retVal = super.getDefaultControllerConfiguration();
+ retVal.put(CommonConstants.Controller.CONTROLLER_SERVICE_AUTO_DISCOVERY, true);
+ return retVal;
+ }
+
+ @Override
+ protected PinotConfiguration getDefaultBrokerConfiguration() {
+ PinotConfiguration config = new PinotConfiguration();
+ config.setProperty(CommonConstants.Broker.BROKER_SERVICE_AUTO_DISCOVERY, true);
+ return config;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir);
+
+ // Start the Pinot cluster
+ startZk();
+ startController();
+ startBrokers(1);
+ startServers(1);
+
+ }
Review comment:
https://github.com/apache/pinot/pull/8197
You triggered my OCD...
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1037147875
High level question: does this affect startup time?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7502ff1) into [master](https://codecov.io/gh/apache/pinot/commit/58f74ce3a9decd02cefeaef79691d9940f509e7f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (58f74ce) will **decrease** coverage by `6.66%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
============================================
- Coverage 71.36% 64.69% -6.67%
+ Complexity 4308 4307 -1
============================================
Files 1623 1579 -44
Lines 84356 82497 -1859
Branches 12654 12455 -199
============================================
- Hits 60197 53372 -6825
- Misses 20023 25311 +5288
+ Partials 4136 3814 -322
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.87% <0.00%> (-0.02%)` | :arrow_down: |
| unittests2 | `14.20% <0.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `89.09% <0.00%> (-3.37%)` | :arrow_down: |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `23.63% <ø> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [373 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [58f74ce...7502ff1](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b28d6dd) into [master](https://codecov.io/gh/apache/pinot/commit/58f74ce3a9decd02cefeaef79691d9940f509e7f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (58f74ce) will **decrease** coverage by `1.25%`.
> The diff coverage is `33.33%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
============================================
- Coverage 71.36% 70.10% -1.26%
Complexity 4308 4308
============================================
Files 1623 1624 +1
Lines 84356 84360 +4
Branches 12654 12655 +1
============================================
- Hits 60197 59140 -1057
- Misses 20023 21129 +1106
+ Partials 4136 4091 -45
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.58% <60.00%> (-0.11%)` | :arrow_down: |
| unittests1 | `67.88% <0.00%> (-0.01%)` | :arrow_down: |
| unittests2 | `14.22% <0.00%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `89.09% <0.00%> (-3.37%)` | :arrow_down: |
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `23.63% <ø> (ø)` | |
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `93.02% <100.00%> (ø)` | |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `81.81% <100.00%> (ø)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-94.74%)` | :arrow_down: |
| ... and [108 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [58f74ce...b28d6dd](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b28d6dd) into [master](https://codecov.io/gh/apache/pinot/commit/58f74ce3a9decd02cefeaef79691d9940f509e7f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (58f74ce) will **decrease** coverage by `6.63%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
============================================
- Coverage 71.36% 64.73% -6.64%
Complexity 4308 4308
============================================
Files 1623 1579 -44
Lines 84356 82479 -1877
Branches 12654 12451 -203
============================================
- Hits 60197 53389 -6808
- Misses 20023 25272 +5249
+ Partials 4136 3818 -318
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.88% <0.00%> (-0.01%)` | :arrow_down: |
| unittests2 | `14.22% <0.00%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `89.09% <0.00%> (-3.37%)` | :arrow_down: |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `23.63% <ø> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [373 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [58f74ce...b28d6dd](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] dongxiaoman commented on a change in pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
dongxiaoman commented on a change in pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#discussion_r805057238
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/api/ServiceAutoDiscoveryFeature.java
##########
@@ -68,8 +69,8 @@
* pinot-integration-tests/src/main/java/org/apache/pinot/broker/integration/tests/BrokerTestAutoLoadedService.java
* </code>
*/
-public class BrokerServiceAutoDiscoveryFeature implements Feature {
- private static final Logger LOGGER = LoggerFactory.getLogger(BrokerServiceAutoDiscoveryFeature.class);
+public class ServiceAutoDiscoveryFeature implements Feature {
Review comment:
A very good point. I added some code (static boolean) to make sure discovery is done once per JVM, and did the integration test that starts Broker+Controller in the same test.
The result shows that only one of the Broker/Controller can find service properly.
It seems that based on Jersey, Services/Features are isolated per `Application` (tied to ResourceConfig) level, so if there are two applications, the discovery has to be done twice.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b28d6dd) into [master](https://codecov.io/gh/apache/pinot/commit/58f74ce3a9decd02cefeaef79691d9940f509e7f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (58f74ce) will **decrease** coverage by `57.13%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
=============================================
- Coverage 71.36% 14.22% -57.14%
+ Complexity 4308 81 -4227
=============================================
Files 1623 1579 -44
Lines 84356 82479 -1877
Branches 12654 12451 -203
=============================================
- Hits 60197 11733 -48464
- Misses 20023 69869 +49846
+ Partials 4136 877 -3259
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.22% <0.00%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `89.09% <0.00%> (-3.37%)` | :arrow_down: |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (-23.64%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1298 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [58f74ce...b28d6dd](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1035705132
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8193](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9638065) into [master](https://codecov.io/gh/apache/pinot/commit/22b17c3391be5d239bee879d5102123b7d385edc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22b17c3) will **decrease** coverage by `6.57%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8193 +/- ##
============================================
- Coverage 71.36% 64.79% -6.58%
- Complexity 4308 4311 +3
============================================
Files 1623 1579 -44
Lines 84365 82488 -1877
Branches 12657 12454 -203
============================================
- Hits 60208 53447 -6761
- Misses 20032 25219 +5187
+ Partials 4125 3822 -303
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.90% <0.00%> (+0.01%)` | :arrow_up: |
| unittests2 | `14.27% <0.00%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `89.09% <0.00%> (-3.37%)` | :arrow_down: |
| [...he/pinot/core/api/ServiceAutoDiscoveryFeature.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hcGkvU2VydmljZUF1dG9EaXNjb3ZlcnlGZWF0dXJlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ker/api/resources/BrokerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2Jyb2tlci9hcGkvcmVzb3VyY2VzL0Jyb2tlckVjaG9XaXRoQXV0b0Rpc2NvdmVyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...api/resources/ControllerEchoWithAutoDiscovery.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvbnRyb2xsZXIvYXBpL3Jlc291cmNlcy9Db250cm9sbGVyRWNob1dpdGhBdXRvRGlzY292ZXJ5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/core/api/AutoLoadedServiceForTest.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtaW50ZWdyYXRpb24tdGVzdHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvYXBpL0F1dG9Mb2FkZWRTZXJ2aWNlRm9yVGVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `23.63% <ø> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [374 more](https://codecov.io/gh/apache/pinot/pull/8193/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [22b17c3...9638065](https://codecov.io/gh/apache/pinot/pull/8193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1038188674
> > High level question: does this affect startup time?
>
> I think this more comes from each customized Feature implementation?
Is there a discovery cost?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin merged pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
richardstartin merged pull request #8193:
URL: https://github.com/apache/pinot/pull/8193
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1038001955
> High level question: does this affect startup time?
I think this more comes from each customized Feature implementation?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on pull request #8193: Enable service discovery in controller too
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on pull request #8193:
URL: https://github.com/apache/pinot/pull/8193#issuecomment-1038001955
> High level question: does this affect startup time?
I think this more comes from each customized Feature implementation?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org