mlbiscoc commented on code in PR #3682:
URL: https://github.com/apache/solr/pull/3682#discussion_r2373207536
##########
solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java:
##########
@@ -387,12 +239,192 @@ public Map<String, Object> getRunningMerges() {
@Override
protected void doAfterFlush() throws IOException {
- if (flushes != null) { // this is null when writer is used only for
snapshot cleanup
- flushes.inc(); // or if mergeTotals == false
+ if (flushesCounter != null) { // this is null when writer is used only for
snapshot cleanup
+ flushesCounter.inc(); // or if mergeTotals == false
}
super.doAfterFlush();
}
+ private void initMetrics(final SolrCore core) {
+ if (solrMetricsContext == null) {
+ solrMetricsContext = core.getSolrMetricsContext().getChildContext(this);
+ }
+ var coreName = core.getName();
+ var baseAttributesBuilder =
+ Attributes.builder()
+ .put(CATEGORY_ATTR, SolrInfoBean.Category.INDEX.toString())
+ .put(CORE_ATTR, coreName);
+ if (core.getCoreContainer().isZooKeeperAware()) {
+ String collectionName = core.getCoreDescriptor().getCollectionName();
+ baseAttributesBuilder
+ .put(COLLECTION_ATTR, collectionName)
+ .put(SHARD_ATTR,
core.getCoreDescriptor().getCloudDescriptor().getShardId())
+ .put(REPLICA_ATTR, Utils.parseMetricsReplicaName(collectionName,
coreName));
+ }
+ var baseAttributes = baseAttributesBuilder.build();
+
+ var mergeTimerBaseMetric =
+ solrMetricsContext.longHistogram(
+ "solr_indexwriter_merge_time", "Time spent merging segments",
OtelUnit.MILLISECONDS);
+
+ majorMergeTimer =
+ new AttributedLongTimer(
+ mergeTimerBaseMetric,
baseAttributes.toBuilder().put(MERGE_TYPE_ATTR, "major").build());
+ minorMergeTimer =
+ new AttributedLongTimer(
+ mergeTimerBaseMetric,
baseAttributes.toBuilder().put(MERGE_TYPE_ATTR, "minor").build());
+
+ mergeErrorsCounter =
+ new AttributedLongCounter(
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_merge_errors", "Number of merge errors"),
+ baseAttributes);
+
+ flushesCounter =
+ new AttributedLongCounter(
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_flushes", "Number of times documents have
been flushed to disk"),
+ baseAttributes);
+
+ var mergesCountBaseMetric =
+ solrMetricsContext.longCounter("solr_indexwriter_merges", "Number of
merge operations");
+ var docsMergedCountBaseMetric =
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_docs_merged", "Number of documents involved in
merge");
+ var docsDeletedCountBasedMetric =
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_docs_deleted", "Number of documents involved in
merge");
+ var segmentsCountBaseMetric =
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_segments_merged", "Number of segments involved
in merge");
+
+ // merge started metrics
+ var majorStartedAttributes =
+ baseAttributes.toBuilder()
+ .put(MERGE_TYPE_ATTR, "major")
+ .put(MERGE_STATE_ATTR, "started")
+ .build();
+ majorMergeStartedMetrics.put(
+ MERGES_METRIC_NAME,
+ new AttributedLongCounter(mergesCountBaseMetric,
majorStartedAttributes));
+ majorMergeStartedMetrics.put(
+ MERGE_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsMergedCountBaseMetric,
majorStartedAttributes));
+ majorMergeStartedMetrics.put(
+ MERGE_DELETED_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsDeletedCountBasedMetric,
majorStartedAttributes));
+ majorMergeStartedMetrics.put(
+ MERGE_SEGMENTS_METRIC_NAME,
+ new AttributedLongCounter(segmentsCountBaseMetric,
majorStartedAttributes));
+
+ var minorStartedAttributes =
+ baseAttributes.toBuilder()
+ .put(MERGE_TYPE_ATTR, "minor")
+ .put(MERGE_STATE_ATTR, "started")
+ .build();
+ minorMergeStartedMetrics.put(
+ MERGES_METRIC_NAME,
+ new AttributedLongCounter(mergesCountBaseMetric,
minorStartedAttributes));
+ minorMergeStartedMetrics.put(
+ MERGE_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsMergedCountBaseMetric,
minorStartedAttributes));
+ minorMergeStartedMetrics.put(
+ MERGE_DELETED_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsDeletedCountBasedMetric,
minorStartedAttributes));
+ minorMergeStartedMetrics.put(
+ MERGE_SEGMENTS_METRIC_NAME,
+ new AttributedLongCounter(segmentsCountBaseMetric,
minorStartedAttributes));
+
+ // merge finished metrics
+ var majorFinishedAttributes =
+ baseAttributes.toBuilder()
+ .put(MERGE_TYPE_ATTR, "major")
+ .put(MERGE_STATE_ATTR, "finished")
+ .build();
+ majorMergeFinishedMetrics.put(
+ MERGES_METRIC_NAME,
+ new AttributedLongCounter(mergesCountBaseMetric,
majorFinishedAttributes));
+ majorMergeFinishedMetrics.put(
+ MERGE_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsMergedCountBaseMetric,
majorFinishedAttributes));
+ majorMergeFinishedMetrics.put(
+ MERGE_DELETED_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsDeletedCountBasedMetric,
majorFinishedAttributes));
+ majorMergeFinishedMetrics.put(
+ MERGE_SEGMENTS_METRIC_NAME,
+ new AttributedLongCounter(segmentsCountBaseMetric,
majorFinishedAttributes));
+
+ var minorFinishedAttributes =
+ baseAttributes.toBuilder()
+ .put(MERGE_TYPE_ATTR, "minor")
+ .put(MERGE_STATE_ATTR, "finished")
+ .build();
+ minorMergeFinishedMetrics.put(
+ MERGES_METRIC_NAME,
+ new AttributedLongCounter(mergesCountBaseMetric,
minorFinishedAttributes));
+ minorMergeFinishedMetrics.put(
+ MERGE_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsMergedCountBaseMetric,
minorFinishedAttributes));
+ minorMergeFinishedMetrics.put(
+ MERGE_DELETED_DOCS_METRIC_NAME,
+ new AttributedLongCounter(docsDeletedCountBasedMetric,
minorFinishedAttributes));
+ minorMergeFinishedMetrics.put(
+ MERGE_SEGMENTS_METRIC_NAME,
+ new AttributedLongCounter(segmentsCountBaseMetric,
minorFinishedAttributes));
+ }
+
+ /**
+ * Updates relevant metrics related to segment merging
+ *
+ * @param numDocs number of documents in merge op
+ * @param numDeletedDocs number of deleted docs in merge op
+ * @param numSegments number of segments in merge op
+ * @param mergeCompleted true if being called post-merge, else false to
signify a merge is about
+ * to start
+ * @param metricTimer an existing timer context for actively running merge
+ * @return timer context for current merge operation
+ */
+ private AttributedLongTimer.MetricTimer updateMergeMetrics(
+ long numDocs,
+ long numDeletedDocs,
+ long numSegments,
+ boolean mergeCompleted,
+ AttributedLongTimer.MetricTimer metricTimer) {
+ if (solrMetricsContext == null) {
+ return null;
+ }
+ boolean isMajorMerge = numDocs - numDeletedDocs > majorMergeDocs;
Review Comment:
This didn't look right. `totalNumDocs` is passed into here and is a net
subtracting the deleted docs but `numDocs` subtracts it again on this boolean
check.
##########
solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java:
##########
@@ -387,12 +239,192 @@ public Map<String, Object> getRunningMerges() {
@Override
protected void doAfterFlush() throws IOException {
- if (flushes != null) { // this is null when writer is used only for
snapshot cleanup
- flushes.inc(); // or if mergeTotals == false
+ if (flushesCounter != null) { // this is null when writer is used only for
snapshot cleanup
+ flushesCounter.inc(); // or if mergeTotals == false
}
super.doAfterFlush();
}
+ private void initMetrics(final SolrCore core) {
+ if (solrMetricsContext == null) {
+ solrMetricsContext = core.getSolrMetricsContext().getChildContext(this);
+ }
+ var coreName = core.getName();
+ var baseAttributesBuilder =
+ Attributes.builder()
+ .put(CATEGORY_ATTR, SolrInfoBean.Category.INDEX.toString())
+ .put(CORE_ATTR, coreName);
+ if (core.getCoreContainer().isZooKeeperAware()) {
+ String collectionName = core.getCoreDescriptor().getCollectionName();
+ baseAttributesBuilder
+ .put(COLLECTION_ATTR, collectionName)
+ .put(SHARD_ATTR,
core.getCoreDescriptor().getCloudDescriptor().getShardId())
+ .put(REPLICA_ATTR, Utils.parseMetricsReplicaName(collectionName,
coreName));
+ }
+ var baseAttributes = baseAttributesBuilder.build();
+
+ var mergeTimerBaseMetric =
+ solrMetricsContext.longHistogram(
+ "solr_indexwriter_merge_time", "Time spent merging segments",
OtelUnit.MILLISECONDS);
+
+ majorMergeTimer =
+ new AttributedLongTimer(
+ mergeTimerBaseMetric,
baseAttributes.toBuilder().put(MERGE_TYPE_ATTR, "major").build());
+ minorMergeTimer =
+ new AttributedLongTimer(
+ mergeTimerBaseMetric,
baseAttributes.toBuilder().put(MERGE_TYPE_ATTR, "minor").build());
+
+ mergeErrorsCounter =
+ new AttributedLongCounter(
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_merge_errors", "Number of merge errors"),
+ baseAttributes);
+
+ flushesCounter =
+ new AttributedLongCounter(
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_flushes", "Number of times documents have
been flushed to disk"),
+ baseAttributes);
+
+ var mergesCountBaseMetric =
+ solrMetricsContext.longCounter("solr_indexwriter_merges", "Number of
merge operations");
+ var docsMergedCountBaseMetric =
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_docs_merged", "Number of documents involved in
merge");
+ var docsDeletedCountBasedMetric =
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_docs_deleted", "Number of documents involved in
merge");
+ var segmentsCountBaseMetric =
+ solrMetricsContext.longCounter(
+ "solr_indexwriter_segments_merged", "Number of segments involved
in merge");
Review Comment:
Can you update the descriptions here instead of the same ones?
##########
solr/core/src/test/org/apache/solr/update/SolrIndexMetricsTest.java:
##########
@@ -53,63 +54,8 @@ private void addDocs() throws Exception {
h.reload();
}
- @Test
- public void testIndexMetricsNoDetails() throws Exception {
Review Comment:
Just to confirm, we delete this test because this is on by default all the
time now?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
