[
https://issues.apache.org/jira/browse/PHOENIX-5980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17776932#comment-17776932
]
ASF GitHub Bot commented on PHOENIX-5980:
-----------------------------------------
virajjasani commented on code in PR #1717:
URL: https://github.com/apache/phoenix/pull/1717#discussion_r1364654214
##########
phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java:
##########
@@ -1584,12 +1588,27 @@ public static MutationMetricQueue.MutationMetric
updateMutationBatchFailureMetri
numDeleteMutationsInBatch++;
}
}
+
+ long totalFailedMutation = numUpsertMutationsInBatch +
numDeleteMutationsInBatch;
+ //TODO: this case is something that should not happen
+ //but still if condition makes sese if this ever happens
+ if (totalFailedMutation < numFailedMutations) {
+ LOGGER.warn(
+ "total failed mutation less than num of failed mutation.
This is not expected.");
+ totalFailedMutation = numFailedMutations;
+ }
+
System.out.println(GLOBAL_MUTATION_BATCH_FAILED_COUNT.getMetric().getValue());
Review Comment:
looks like this was for your debugging purpose, feel free to remove :)
##########
phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java:
##########
@@ -1584,12 +1588,27 @@ public static MutationMetricQueue.MutationMetric
updateMutationBatchFailureMetri
numDeleteMutationsInBatch++;
}
}
+
+ long totalFailedMutation = numUpsertMutationsInBatch +
numDeleteMutationsInBatch;
+ //TODO: this case is something that should not happen
+ //but still if condition makes sese if this ever happens
+ if (totalFailedMutation < numFailedMutations) {
+ LOGGER.warn(
+ "total failed mutation less than num of failed mutation.
This is not expected.");
+ totalFailedMutation = numFailedMutations;
+ }
+
System.out.println(GLOBAL_MUTATION_BATCH_FAILED_COUNT.getMetric().getValue());
+
+ GLOBAL_MUTATION_BATCH_FAILED_COUNT.update(
+ allDeletesMutations && !isTransactional ?
numDeleteMutationsInBatch :
+ totalFailedMutation);
Review Comment:
we can extract the logic and keep value in one variable so the same variable
can be used for both `GLOBAL_MUTATION_BATCH_FAILED_COUNT.update` and
`MutationMetricQueue.MutationMetric`
> MUTATION_BATCH_FAILED_SIZE metric is incorrectly updated for failing delete
> mutations
> -------------------------------------------------------------------------------------
>
> Key: PHOENIX-5980
> URL: https://issues.apache.org/jira/browse/PHOENIX-5980
> Project: Phoenix
> Issue Type: Bug
> Affects Versions: 4.15.0
> Reporter: Chinmay Kulkarni
> Assignee: Divneet Kaur
> Priority: Major
> Labels: metrics, phoenix-hardening, quality-improvement
>
> In the conn.commit() path, we get the number of mutations that failed to be
> committed in the catch block of MutationState.sendMutations() (see
> [here|https://github.com/apache/phoenix/blob/dcc88af8acc2ba8df10d2e9d498ab3646fdf0a78/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java#L1195-L1198]).
>
> In case of delete mutations, the uncommittedStatementIndexes.length always
> resolves to 1 and we always update the metric value by 1 in this case, even
> though the actual mutation list corresponds to multiple DELETE mutations
> which failed. In case of upserts, using unCommittedStatementIndexes.length is
> fine since each upsert query corresponds to 1 Put. We should fix the logic
> for deletes/mixed delete + upsert mutation batch failures.
> This wrong value is propagated to global client metrics as well as
> MutationMetricQueue metrics.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
