You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "suyashpatel98 (via GitHub)" <gi...@apache.org> on 2024/02/09 06:15:17 UTC
[PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
suyashpatel98 opened a new pull request, #12389:
URL: https://github.com/apache/pinot/pull/12389
Add uptime and start-time endpoints for server, broker, minion and controller. Verified all endpoints work as expected.
Following is sample output obtained while testing:
```
[ 7:47PM ] [ suyash@Suyashs-MacBook-Air:~/Desktop/work/open-source/workspace/pinot(12298-new-api-component-uptime✗) ]
$ curl http://localhost:8000/uptime
Uptime: 81 seconds%
[ 7:47PM ] [ suyash@Suyashs-MacBook-Air:~/Desktop/work/open-source/workspace/pinot(12298-new-api-component-uptime✗) ]
$ curl http://localhost:9000/uptime
Uptime: 125 seconds%
[ 8:51PM ] [ suyash@Suyashs-MacBook-Air:~/Desktop/work/open-source/workspace/pinot(12298-new-api-component-uptime✗) ]
$ curl http://localhost:8000/uptime
Uptime: 127 seconds%
[ 8:51PM ] [ suyash@Suyashs-MacBook-Air:~/Desktop/work/open-source/workspace/pinot(12298-new-api-component-uptime✗) ]
$ curl http://localhost:6000/uptime
Uptime: 128 seconds%
[ 8:51PM ] [ suyash@Suyashs-MacBook-Air:~/Desktop/work/open-source/workspace/pinot(12298-new-api-component-uptime✗) ]
$ curl http://localhost:7500/uptime
Uptime: 149 seconds%
[ 8:51PM ] [ suyash@Suyashs-MacBook-Air:~/Desktop/work/open-source/workspace/pinot(12298-new-api-component-uptime✗) ]
$ curl http://localhost:7500/start-time
Pinot server started at: 2024-02-09T04:49:17.096988Z. Pinot server status is GOOD%
```
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1495126555
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -45,19 +47,26 @@
import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
-
+@Singleton
@Api(tags = "Health", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
@Path("/")
public class PinotBrokerHealthCheck {
- @Inject
- @Named(BrokerAdminApiApplication.BROKER_INSTANCE_ID)
+
private String _instanceId;
- @Inject
private BrokerMetrics _brokerMetrics;
+ private Instant _startTime;
Review Comment:
Can you try to follow the Inject pattern for startTime as well?
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1495127223
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -79,4 +88,40 @@ public String getBrokerHealth() {
Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
throw new WebApplicationException(errMessage, response);
}
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("uptime")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker uptime")
+ public String getUptime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ if (status == ServiceStatus.Status.GOOD) {
Review Comment:
I don't think we need to check service status here.
Update can be just a seconds number
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1495134669
##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/BrokerAdminApiApplication.java:
##########
@@ -109,6 +112,8 @@ protected void configure() {
bind(brokerConf.getProperty(CommonConstants.Broker.CONFIG_OF_BROKER_ID)).named(BROKER_INSTANCE_ID);
bind(serverRoutingStatsManager).to(ServerRoutingStatsManager.class);
bind(accessFactory).to(AccessControlFactory.class);
+ bind(instanceId).named(BrokerAdminApiApplication.BROKER_INSTANCE_ID);
+ bind(pinotBrokerHealthCheck).to(PinotBrokerHealthCheck.class);
Review Comment:
no need to bind pinotBrokerHealthCheck, just bind instanceId and startTime.
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1494706069
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -45,19 +47,26 @@
import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
-
+@Singleton
@Api(tags = "Health", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
@Path("/")
public class PinotBrokerHealthCheck {
- @Inject
- @Named(BrokerAdminApiApplication.BROKER_INSTANCE_ID)
+
private String _instanceId;
- @Inject
Review Comment:
why not inject?
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1494706740
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -45,19 +47,26 @@
import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
-
+@Singleton
@Api(tags = "Health", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
@Path("/")
public class PinotBrokerHealthCheck {
- @Inject
- @Named(BrokerAdminApiApplication.BROKER_INSTANCE_ID)
Review Comment:
this should be provided by the bind value
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1495134133
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -79,4 +88,40 @@ public String getBrokerHealth() {
Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
throw new WebApplicationException(errMessage, response);
}
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("uptime")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker uptime")
+ public String getUptime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ if (status == ServiceStatus.Status.GOOD) {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_OK_CALLS, 1);
+ Instant now = Instant.now();
+ Duration uptime = Duration.between(_startTime, now);
+ return "Uptime: " + uptime.getSeconds() + " seconds";
+ }
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_BAD_CALLS, 1);
+ String errMessage = String.format("Cannot tell Pinot broker uptime. Pinot broker status is %s", status);
+ Response response =
+ Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
+ throw new WebApplicationException(errMessage, response);
+ }
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("start-time")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker start time")
+ public String getStartTime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ if (status == ServiceStatus.Status.GOOD) {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_OK_CALLS, 1);
+ } else {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_BAD_CALLS, 1);
+ }
+ String returnMessage = String.format("Pinot broker started at: %s. Pinot broker status is %s", _startTime, status);
Review Comment:
just return _startTime epoch value is good
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1495127484
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -79,4 +88,40 @@ public String getBrokerHealth() {
Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
throw new WebApplicationException(errMessage, response);
}
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("uptime")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker uptime")
+ public String getUptime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ if (status == ServiceStatus.Status.GOOD) {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_OK_CALLS, 1);
+ Instant now = Instant.now();
+ Duration uptime = Duration.between(_startTime, now);
+ return "Uptime: " + uptime.getSeconds() + " seconds";
Review Comment:
just `return uptime.getSeconds();` is good enough
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1495133985
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -79,4 +88,40 @@ public String getBrokerHealth() {
Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
throw new WebApplicationException(errMessage, response);
}
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("uptime")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker uptime")
+ public String getUptime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ if (status == ServiceStatus.Status.GOOD) {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_OK_CALLS, 1);
+ Instant now = Instant.now();
+ Duration uptime = Duration.between(_startTime, now);
+ return "Uptime: " + uptime.getSeconds() + " seconds";
+ }
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_BAD_CALLS, 1);
+ String errMessage = String.format("Cannot tell Pinot broker uptime. Pinot broker status is %s", status);
+ Response response =
+ Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
+ throw new WebApplicationException(errMessage, response);
+ }
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("start-time")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker start time")
+ public String getStartTime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
Review Comment:
no need to check service status = good.
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1494708735
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -45,19 +47,26 @@
import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
-
+@Singleton
Review Comment:
This may not be a singleton, we may start multiple broker instances in same JVM
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1494708735
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -45,19 +47,26 @@
import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
-
+@Singleton
Review Comment:
This is not a singleton, we may start multiple broker instances in same JVM
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #12389:
URL: https://github.com/apache/pinot/pull/12389#issuecomment-1953896144
Thanks for raising the PR! I think the concept works well. Just need to use the right mechanism to ingest variables.
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12389:
URL: https://github.com/apache/pinot/pull/12389#issuecomment-1952784456
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `108 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`ed6761a`)](https://app.codecov.io/gh/apache/pinot/commit/ed6761a982d4666c68b8fcda598a84571446c2ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.65% compared to head [(`8e20657`)](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 34.83%.
> Report is 57 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ller/api/resources/PinotControllerHealthCheck.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90Q29udHJvbGxlckhlYWx0aENoZWNrLmphdmE=) | 0.00% | [23 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...inot/server/api/resources/HealthCheckResource.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9IZWFsdGhDaGVja1Jlc291cmNlLmphdmE=) | 0.00% | [23 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...t/broker/api/resources/PinotBrokerHealthCheck.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckhlYWx0aENoZWNrLmphdmE=) | 0.00% | [22 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...inot/minion/api/resources/HealthCheckResource.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vYXBpL3Jlc291cmNlcy9IZWFsdGhDaGVja1Jlc291cmNlLmphdmE=) | 0.00% | [22 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...g/apache/pinot/server/api/AdminApiApplication.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL0FkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...apache/pinot/controller/BaseControllerStarter.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9CYXNlQ29udHJvbGxlclN0YXJ0ZXIuamF2YQ==) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...apache/pinot/minion/MinionAdminApiApplication.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vTWluaW9uQWRtaW5BcGlBcHBsaWNhdGlvbi5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../pinot/server/starter/helix/BaseServerStarter.java](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9CYXNlU2VydmVyU3RhcnRlci5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12389 +/- ##
=============================================
- Coverage 61.65% 34.83% -26.83%
+ Complexity 1146 6 -1140
=============================================
Files 2421 2352 -69
Lines 131842 129179 -2663
Branches 20344 19995 -349
=============================================
- Hits 81289 44995 -36294
- Misses 44594 80965 +36371
+ Partials 5959 3219 -2740
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.61%)` | :arrow_down: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.83% <0.00%> (-26.71%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.81% <0.00%> (-26.83%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.80% <0.00%> (-26.71%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.83% <0.00%> (-26.83%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.72% <ø> (-14.93%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.72% <ø> (-0.06%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12389/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12389?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1495134133
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerHealthCheck.java:
##########
@@ -79,4 +88,40 @@ public String getBrokerHealth() {
Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
throw new WebApplicationException(errMessage, response);
}
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("uptime")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker uptime")
+ public String getUptime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ if (status == ServiceStatus.Status.GOOD) {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_OK_CALLS, 1);
+ Instant now = Instant.now();
+ Duration uptime = Duration.between(_startTime, now);
+ return "Uptime: " + uptime.getSeconds() + " seconds";
+ }
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_BAD_CALLS, 1);
+ String errMessage = String.format("Cannot tell Pinot broker uptime. Pinot broker status is %s", status);
+ Response response =
+ Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(errMessage).build();
+ throw new WebApplicationException(errMessage, response);
+ }
+
+ @GET
+ @Produces(MediaType.TEXT_PLAIN)
+ @Path("start-time")
+ @Authorize(targetType = TargetType.CLUSTER, action = Actions.Cluster.GET_HEALTH)
+ @ApiOperation(value = "Get broker start time")
+ public String getStartTime() {
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ if (status == ServiceStatus.Status.GOOD) {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_OK_CALLS, 1);
+ } else {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.HEALTHCHECK_BAD_CALLS, 1);
+ }
+ String returnMessage = String.format("Pinot broker started at: %s. Pinot broker status is %s", _startTime, status);
Review Comment:
just return _startTime epoch value is good enough.
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "suyashpatel98 (via GitHub)" <gi...@apache.org>.
suyashpatel98 commented on code in PR #12389:
URL: https://github.com/apache/pinot/pull/12389#discussion_r1483917499
##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/BrokerAdminApiApplication.java:
##########
@@ -109,6 +112,8 @@ protected void configure() {
bind(brokerConf.getProperty(CommonConstants.Broker.CONFIG_OF_BROKER_ID)).named(BROKER_INSTANCE_ID);
bind(serverRoutingStatsManager).to(ServerRoutingStatsManager.class);
bind(accessFactory).to(AccessControlFactory.class);
+ bind(instanceId).named(BrokerAdminApiApplication.BROKER_INSTANCE_ID);
+ bind(pinotBrokerHealthCheck).to(PinotBrokerHealthCheck.class);
Review Comment:
This is to register objects with the DI framework because these were instantiated manually. As mentioned in the description in order to preserve the start time, it is essential to create an object at start time and ensure that it exists through out the lifecycle of the application
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "suyashpatel98 (via GitHub)" <gi...@apache.org>.
suyashpatel98 commented on PR #12389:
URL: https://github.com/apache/pinot/pull/12389#issuecomment-1968320496
Closing this PR because the history looks messy. Will create a new one
--
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
Re: [PR] Add uptime and start-time endpoints for server, broker, minion and controller [pinot]
Posted by "suyashpatel98 (via GitHub)" <gi...@apache.org>.
suyashpatel98 closed pull request #12389: Add uptime and start-time endpoints for server, broker, minion and controller
URL: https://github.com/apache/pinot/pull/12389
--
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