You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/12 23:15:17 UTC
[GitHub] [lucene-solr] dsmiley opened a new pull request #2198: SOLR-15081: Metrics for core: isLeader, status
dsmiley opened a new pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198
https://issues.apache.org/jira/browse/SOLR-15081
Copying the description:
> The core level metrics hold some interesting information, but I don't see information pertaining to the SolrCloud status of the core. In particular, I'd like to see the leader status here, and also the replica state. The use-case I have in mind is enabling the Prometheus Exporter to get the doc count (and maybe other basics) of only the leader replicas, thereby counting unique documents instead of a fully replicated figure. This is an approximation to doing a match-all-docs query on all collections, but is a more sound approach when one has orders of magnitude more collections than nodes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556169620
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
Review comment:
I removed the check for CoreContainer nullness as it seemed impossible for it to be null. SolrCores hang off of a CoreContainer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] megancarey commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
megancarey commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556161229
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
Review comment:
Since we do reference coreContainer on like 1215 should we keep this check?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley merged pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
dsmiley merged pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556446332
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
+ // SEE org.apache.solr.handler.admin.CoreAdminOperation.getCoreStatus
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ Replica leaderReplica = docColl.getLeader(cd.getShardId());
+ return leaderReplica.getName().equals(cd.getCoreNodeName());
+ }, true, "isLeader", Category.CORE.toString());
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ final Replica myReplica = docColl.getReplica(cd.getCoreNodeName());
+ return Objects.requireNonNullElse(myReplica.getState().toString(), parentContext.nullString());
Review comment:
Similarly `cloudDescriptor.getLastPublished()`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] muse-dev[bot] commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
muse-dev[bot] commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556192502
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
+ // SEE org.apache.solr.handler.admin.CoreAdminOperation.getCoreStatus
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ Replica leaderReplica = docColl.getLeader(cd.getShardId());
Review comment:
*NULLPTR_DEREFERENCE:* accessing memory that is the null pointer on line 1215 indirectly during the call to `ClusterState.getCollectionOrNull(...)`.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
+ // SEE org.apache.solr.handler.admin.CoreAdminOperation.getCoreStatus
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ Replica leaderReplica = docColl.getLeader(cd.getShardId());
+ return leaderReplica.getName().equals(cd.getCoreNodeName());
Review comment:
*NULLPTR_DEREFERENCE:* accessing memory that is the null pointer on line 1216 indirectly during the call to `DocCollection.getLeader(...)`.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
+ // SEE org.apache.solr.handler.admin.CoreAdminOperation.getCoreStatus
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ Replica leaderReplica = docColl.getLeader(cd.getShardId());
+ return leaderReplica.getName().equals(cd.getCoreNodeName());
+ }, true, "isLeader", Category.CORE.toString());
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ final Replica myReplica = docColl.getReplica(cd.getCoreNodeName());
Review comment:
*NULLPTR_DEREFERENCE:* accessing memory that is the null pointer on line 1220 indirectly during the call to `ClusterState.getCollectionOrNull(...)`.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
+ // SEE org.apache.solr.handler.admin.CoreAdminOperation.getCoreStatus
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ Replica leaderReplica = docColl.getLeader(cd.getShardId());
+ return leaderReplica.getName().equals(cd.getCoreNodeName());
+ }, true, "isLeader", Category.CORE.toString());
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ final Replica myReplica = docColl.getReplica(cd.getCoreNodeName());
+ return Objects.requireNonNullElse(myReplica.getState().toString(), parentContext.nullString());
Review comment:
*NULLPTR_DEREFERENCE:* accessing memory that is the null pointer on line 1221 indirectly during the call to `DocCollection.getReplica(...)`.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
+ // SEE org.apache.solr.handler.admin.CoreAdminOperation.getCoreStatus
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ Replica leaderReplica = docColl.getLeader(cd.getShardId());
+ return leaderReplica.getName().equals(cd.getCoreNodeName());
+ }, true, "isLeader", Category.CORE.toString());
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
+ final Replica myReplica = docColl.getReplica(cd.getCoreNodeName());
+ return Objects.requireNonNullElse(myReplica.getState().toString(), parentContext.nullString());
Review comment:
*NULL_DEREFERENCE:* object `myReplica` last assigned on line 1221 could be null and is dereferenced at line 1222.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556258609
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
Review comment:
I wrote these notes and code months ago, shelved it and nearly forgotten, and today I remembered it and submitted it. The TODO here was kind of a note-to-self that can be removed. I'd welcome anyone's thoughts on this though. There appears, to me, to be overlap in scope between metrics and "status" type requests. Years ago I thought Metrics was just numbers, but lately I've seen it can have all sorts of strings and basically completely compete with "status".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] HoustonPutman commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556646373
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
Review comment:
I don't think it's inherently wrong to have overlap between status and metrics, as long as the overlapping information is coming from the same source.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556449504
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
Review comment:
Yeah, metrics today overlap a lot with "status" requests... something to clean up in 9x.
Initially the metrics API wasn't able to properly report complex values (esp. when reported via JMX) but this has been fixed around 7.0 or so - support for non-numeric values had to be added specifically to report things like paths, non-numeric state, etc. and for complex properties like eg. system properties, caches, etc. Now it can report basically anything you want.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on a change in pull request #2198: SOLR-15081: Metrics for core: isLeader, status
Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2198:
URL: https://github.com/apache/lucene-solr/pull/2198#discussion_r556445628
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1202,26 +1203,26 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString());
parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString());
- if (coreContainer != null) {
- final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
- if (cd != null) {
- parentContext.gauge(() -> {
- if (cd.getCollectionName() != null) {
- return cd.getCollectionName();
- } else {
- return parentContext.nullString();
- }
- }, true, "collection", Category.CORE.toString());
- parentContext.gauge(() -> {
- if (cd.getShardId() != null) {
- return cd.getShardId();
- } else {
- return parentContext.nullString();
- }
- }, true, "shard", Category.CORE.toString());
- }
+ final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor();
+ if (cd != null) {
+ // TODO
+ parentContext.gauge(cd::getCollectionName, true, "collection", Category.CORE.toString());
+ parentContext.gauge(() -> Objects.requireNonNullElse(cd.getShardId(), parentContext.nullString()), true, "shard", Category.CORE.toString());
+ //TODO should this instead be in a core status, or a metric? When do we use which?
+ // SEE org.apache.solr.handler.admin.CoreAdminOperation.getCoreStatus
+ parentContext.gauge(() -> {
+ DocCollection docColl = coreContainer.getZkController().getZkStateReader().getClusterState().getCollectionOrNull(cd.getCollectionName(), true);
Review comment:
Maybe use `cloudDescriptor.isLeader()`, it's simpler...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org