liangkaiwen commented on code in PR #3416:
URL: https://github.com/apache/solr/pull/3416#discussion_r2229636794
##########
solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java:
##########
@@ -269,54 +204,22 @@ protected void merge(MergePolicy.OneMerge merge) throws
IOException {
String segString = merge.segString();
long totalNumDocs = merge.totalNumDocs();
runningMerges.put(segString, totalNumDocs);
- if (!mergeTotals) {
- try {
- super.merge(merge);
- } finally {
- runningMerges.remove(segString);
- }
- return;
- }
long deletedDocs = 0;
for (SegmentCommitInfo info : merge.segments) {
totalNumDocs -= info.getDelCount();
deletedDocs += info.getDelCount();
}
- boolean major = totalNumDocs > majorMergeDocs;
int segmentsCount = merge.segments.size();
- Timer.Context context;
- if (major) {
- runningMajorMerges.incrementAndGet();
- runningMajorMergesDocs.addAndGet(totalNumDocs);
- runningMajorMergesSegments.addAndGet(segmentsCount);
- if (mergeDetails) {
- majorMergedDocs.mark(totalNumDocs);
- majorDeletedDocs.mark(deletedDocs);
- }
- context = majorMerge.time();
- } else {
- runningMinorMerges.incrementAndGet();
- runningMinorMergesDocs.addAndGet(totalNumDocs);
- runningMinorMergesSegments.addAndGet(segmentsCount);
- context = minorMerge.time();
- }
+ Timer.Context context =
+ updateMergeMetrics(totalNumDocs, deletedDocs, segmentsCount, false,
null);
try {
super.merge(merge);
} catch (Throwable t) {
- mergeErrors.inc();
+ mergeErrorsCounter.inc();
throw t;
} finally {
runningMerges.remove(segString);
- context.stop();
- if (major) {
- runningMajorMerges.decrementAndGet();
- runningMajorMergesDocs.addAndGet(-totalNumDocs);
- runningMajorMergesSegments.addAndGet(-segmentsCount);
- } else {
- runningMinorMerges.decrementAndGet();
- runningMinorMergesDocs.addAndGet(-totalNumDocs);
- runningMinorMergesSegments.addAndGet(-segmentsCount);
- }
+ updateMergeMetrics(totalNumDocs, deletedDocs, segmentsCount, true,
context);
Review Comment:
No, it shouldn't be negative because this implementation no longer tracks
the state of a current merge. The previous implementation uses negatives
because the merge has finished therefore there are "no documents being
currently merged"
--
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]
