[
https://issues.apache.org/jira/browse/PHOENIX-5980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17777435#comment-17777435
]
ASF GitHub Bot commented on PHOENIX-5980:
-----------------------------------------
Divneet18 commented on code in PR #1717:
URL: https://github.com/apache/phoenix/pull/1717#discussion_r1365974852
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/MutationBatchFailedStateMetricWithAllUpsertIT.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.end2end;
+
+import static
org.apache.phoenix.monitoring.MetricType.DELETE_BATCH_FAILED_SIZE;
+import static
org.apache.phoenix.monitoring.MetricType.UPSERT_BATCH_FAILED_SIZE;
+import static
org.apache.phoenix.monitoring.MetricType.MUTATION_BATCH_FAILED_SIZE;
+import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
+import static
org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_BATCH_FAILED_COUNT;
+
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Properties;
+
+import org.apache.phoenix.end2end.index.ImmutableIndexIT;
+import org.apache.phoenix.hbase.index.Indexer;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.monitoring.MetricType;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.thirdparty.com.google.common.collect.Maps;
+import org.apache.phoenix.util.PropertiesUtil;
+import org.apache.phoenix.util.ReadOnlyProps;
+import org.apache.phoenix.util.TestUtil;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+
+@Category(NeedsOwnMiniClusterTest.class)
+@RunWith(Parameterized.class)
+public class MutationBatchFailedStateMetricWithAllUpsertIT extends
ParallelStatsDisabledIT {
+ String create_table =
+ "CREATE TABLE IF NOT EXISTS %s(ID VARCHAR NOT NULL PRIMARY KEY,
VAL1 INTEGER, VAL2 INTEGER)";
+ String indexName = generateUniqueName();
+ String create_index = "CREATE INDEX " + indexName + " ON %s(VAL1 DESC)
INCLUDE (VAL2)";
+ String upsertStatement = "UPSERT INTO %s VALUES(?, ?, ?)";
+ String deleteTableName = generateUniqueName();
+ private final boolean transactional;
+ private String transactionProvider;
Review Comment:
The reason the tests are in three different classes is because we are
testing for global metrics. every time a test is run and for every parameter
the global metric always gets added on to the previous value rather than
reseting to 0 (like mutation metrics does). Hence the actual answers for the
metrics were different than expected and would change based on the way tests
are run and sometimes the order too. To avoid that, we came up with making
different ITs. However, I will make a superclass to avoid the recurring lines
of code.
> 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)
