You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "sk0x50 (via GitHub)" <gi...@apache.org> on 2025/07/07 11:29:23 UTC
[PR] IGNITE-25844 Add transactions metric source [ignite-3]
sk0x50 opened a new pull request, #6206:
URL: https://github.com/apache/ignite-3/pull/6206
https://issues.apache.org/jira/browse/IGNITE-25844
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2205045400
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadOnlyTransactionImpl.java:
##########
@@ -147,7 +147,7 @@ public CompletableFuture<Void> finish(boolean commit, HybridTimestamp executionT
txFuture.complete(null);
- ((TxManagerImpl) txManager).completeReadOnlyTransactionFuture(new TxIdAndTimestamp(readTimestamp, id()), timeoutExceeded);
+ ((TxManagerImpl) txManager).completeReadOnlyTransactionFuture(commit, new TxIdAndTimestamp(readTimestamp, id()), timeoutExceeded);
Review Comment:
Should it be commitIntent? Minor, up to you.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2207336542
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -712,6 +717,8 @@ public CompletableFuture<Void> finish(
)
).thenAccept(unused -> {
if (localNodeId.equals(finishingStateMeta.txCoordinatorId())) {
+ txMetrics.onReadWriteTransactionFinished(txId, commitIntent);
Review Comment:
Nice ctach! This need to be fixed!
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 merged PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2204224447
##########
modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/ItTransactionMetricsTest.java:
##########
@@ -0,0 +1,256 @@
+/*
+ * 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.ignite.internal.tx;
+
+import static org.apache.ignite.internal.AssignmentsTestUtils.awaitAssignmentsStabilizationOnDefaultZone;
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.metrics.LongMetric;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Tests transaction metrics.
+ */
+public class ItTransactionMetricsTest extends ClusterPerClassIntegrationTest {
+ public static final String TABLE_NAME = "test_table_name";
+
+ @Override
+ protected int initialNodes() {
+ return 2;
+ }
+
+ @BeforeAll
+ void createTable() throws Exception {
+ sql("CREATE TABLE " + TABLE_NAME + " (id INT PRIMARY KEY, val VARCHAR)");
+
+ awaitAssignmentsStabilizationOnDefaultZone(CLUSTER.aliveNode());
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param nodeIndex Node index to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(int nodeIndex) {
+ return keyValueView(CLUSTER.node(nodeIndex));
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param node Node to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(Ignite node) {
+ return node.tables().table(TABLE_NAME).keyValueView(Integer.class, String.class);
+ }
+
+ /**
+ * Returns a snapshot of transaction metrics for the given node.
+ *
+ * @param nodeIndex Node index to capture transaction metrics.
+ * @return Snapshot of transaction metrics.
+ */
+ private Map<String, Long> metricValues(int nodeIndex) {
+ var values = new HashMap<String, Long>();
+
+ MetricSet txMetrics = unwrapIgniteImpl(node(nodeIndex))
+ .metricManager()
+ .metricSnapshot()
+ .metrics()
+ .get(TransactionMetricsSource.SOURCE_NAME);
+
+ txMetrics.iterator().forEachRemaining(metric -> {
+ if (metric instanceof LongMetric) {
+ values.put(metric.name(), ((LongMetric) metric).value());
+ }
+ });
+
+ return values;
+ }
+
+ private static void testMetricValues(Map<String, Long> initial, Map<String, Long> actual, String... ignored) {
+ assertThat("Number of metrics should be the same.", initial.size(), is(actual.size()));
+
+ var exclude = Set.of(ignored);
+
+ for (Map.Entry<String, Long> e : initial.entrySet()) {
+ if (!exclude.contains(e.getKey())) {
+ assertThat("Metric name = " + e.getKey(), actual.get(e.getKey()), is(e.getValue()));
+ }
+ }
+ }
+
+ /**
+ * Tests that TotalCommits and RoCommits are incremented when a read only transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadOnlyTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(tx, 12);
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits are incremented when a read write transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadWriteTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin();
+ keyValueView(0).put(tx, 12, "value");
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RwCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits/RoCommits are incremented when an "empty" transaction successfully committed.
+ * Empty means that there are no entries enlisted into the transaction.
+ */
+ @Test
+ void testCommitEmptyTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ rwTx.commit();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ roTx.commit();
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits, RwCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 2));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalRollbacks and RwRollbacks/RoRollbacks are incremented when a transaction rolled back.
+ */
+ @Test
+ void testRollbackTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ keyValueView(0).put(rwTx, 12, "value");
+ rwTx.rollback();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(roTx, 12);
+ roTx.rollback();
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalRollbacks, RwRollbacks and RoRollbacks.
+ testMetricValues(metrics0, actualMetrics0, "TotalRollbacks", "RwRollbacks", "RoRollbacks");
+
+ assertThat(actualMetrics0.get("TotalRollbacks"), is(metrics0.get("TotalRollbacks") + 2));
+ assertThat(actualMetrics0.get("RwRollbacks"), is(metrics0.get("RwRollbacks") + 1));
+ assertThat(actualMetrics0.get("RoRollbacks"), is(metrics0.get("RoRollbacks") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RoCommits are incremented when a SQL engine is used.
+ */
+ @Test
+ void testSqlImplicitTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ sql(0, "select * from " + TABLE_NAME);
Review Comment:
Updated `testSqlTransaction` in order to cover implicit/explicit and ro/rw transactions.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2204216593
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadOnlyTransactionImpl.java:
##########
@@ -135,7 +136,12 @@ public CompletableFuture<Void> rollbackTimeoutExceededAsync() {
}
@Override
- public CompletableFuture<Void> finish(boolean commit, HybridTimestamp executionTimestamp, boolean full, boolean timeoutExceeded) {
+ public CompletableFuture<Void> finish(
+ boolean commit,
+ @Nullable HybridTimestamp executionTimestamp,
Review Comment:
I just aligned the definition with the base class. That was wrong. Reverted this change.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -1148,11 +1158,16 @@ public CompletableFuture<Void> executeWriteIntentSwitchAsync(Runnable runnable)
return runAsync(runnable, writeIntentSwitchPool);
}
- void completeReadOnlyTransactionFuture(TxIdAndTimestamp txIdAndTimestamp, boolean timeoutExceeded) {
- finishedTxs.add(1);
-
+ void completeReadOnlyTransactionFuture(
+ boolean commit,
+ boolean implicit,
Review Comment:
Removed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2207335942
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/metrics/TransactionMetricsSource.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.ignite.internal.tx.metrics;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.ignite.internal.tx.TransactionIds.beginTimestamp;
+
+import java.util.List;
+import java.util.UUID;
+import org.apache.ignite.internal.hlc.ClockService;
+import org.apache.ignite.internal.metrics.AbstractMetricSource;
+import org.apache.ignite.internal.metrics.DistributionMetric;
+import org.apache.ignite.internal.metrics.LongAdderMetric;
+import org.apache.ignite.internal.metrics.Metric;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource.Holder;
+import org.jetbrains.annotations.TestOnly;
+
+/**
+ * Transaction metric source, that contains a set of transaction metrics.
+ **/
+public class TransactionMetricsSource extends AbstractMetricSource<Holder> {
+ /** Histogram buckets for duration metrics in milliseconds. */
+ private static final long[] HISTOGRAM_BUCKETS = new long[] {
+ MILLISECONDS.convert(1, MILLISECONDS),
+ MILLISECONDS.convert(10, MILLISECONDS),
+ MILLISECONDS.convert(100, MILLISECONDS),
+ MILLISECONDS.convert(250, MILLISECONDS),
+ MILLISECONDS.convert(500, MILLISECONDS),
+ MILLISECONDS.convert(1, SECONDS),
+ MILLISECONDS.convert(5, SECONDS)
+ };
+
+ /** Source name. */
+ public static final String SOURCE_NAME = "transactions";
+
+ /** Clock service to calculate a timestamp for rolled back transactions. */
+ private final ClockService clockService;
+
+ /**
+ * Creates a new instance of {@link TransactionMetricsSource}.
+ */
+ public TransactionMetricsSource(ClockService clockService) {
+ super(SOURCE_NAME, "Transaction metrics.");
+
+ this.clockService = clockService;
+ }
+
+ /**
+ * Updates read-write related metrics.
+ *
+ * @param transactionId Transaction identifier.
+ * @param commit {@code true} if a transaction was committed, and {@code false} otherwise.
+ */
+ public void onReadWriteTransactionFinished(UUID transactionId, boolean commit) {
+ Holder holder = holder();
+
+ if (holder != null) {
+ holder.rwDuration.add(calculateTransactionDuration(transactionId));
+
+ holder.activeTransactions.decrement();
+
+ if (commit) {
+ holder.totalCommits.increment();
+ holder.rwCommits.increment();
+ } else {
+ holder.totalRollbacks.increment();
+ holder.rwRollbacks.increment();
+ }
+ }
+ }
+
+ /**
+ * Updates read-only related metrics.
+ *
+ * @param transactionId Transaction identifier.
+ * @param commit {@code true} if a transaction was committed, and {@code false} otherwise.
+ */
+ public void onReadOnlyTransactionFinished(UUID transactionId, boolean commit) {
+ Holder holder = holder();
+
+ if (holder != null) {
+ holder.roDuration.add(calculateTransactionDuration(transactionId));
+
+ holder.activeTransactions.decrement();
+
+ if (commit) {
+ holder.totalCommits.increment();
+ holder.roCommits.increment();
+ } else {
+ holder.totalRollbacks.increment();
+ holder.roRollbacks.increment();
+ }
+ }
+ }
+
+ /**
+ * Tracks a number of active transactions.
+ */
+ public void onTransactionStarted() {
+ Holder holder = holder();
+
+ if (holder != null) {
+ holder.activeTransactions.increment();
+ }
+ }
+
+ /**
+ * Returns a number of active transactions.
+ * If this metric source is not enabled, then always returns {@code 0}.
+ *
+ * @return Number of active transactions.
+ */
+ public long activeTransactions() {
+ Holder holder = holder();
+
+ if (holder != null) {
+ return holder.activeTransactions.value();
+ }
+
+ return 0L;
+ }
+
+ /**
+ * Returns a number of finished transactions.
+ * If this metric source is not enabled, then always returns {@code 0}.
+ *
+ * @return Number of finished transactions.
+ */
+ public long finishedTransactions() {
+ Holder holder = holder();
+
+ if (holder != null) {
+ return holder.totalCommits.value() + holder.totalRollbacks.value();
+ }
+
+ return 0L;
+ }
+
+ @Override
+ protected Holder createHolder() {
+ return new Holder();
+ }
+
+ private long calculateTransactionDuration(UUID transactionId) {
+ return clockService.currentLong() - beginTimestamp(transactionId).getPhysical();
+ }
+
+ /** Holder. */
+ protected static class Holder implements AbstractMetricSource.Holder<Holder> {
+ private final LongAdderMetric totalCommits = new LongAdderMetric(
+ "TotalCommits",
+ "Total number of commits.");
+
+ private final LongAdderMetric totalRollbacks = new LongAdderMetric(
+ "TotalRollbacks",
+ "Total number of rollbacks.");
+
+ private final LongAdderMetric rwCommits = new LongAdderMetric(
+ "RwCommits",
+ "Total number of read-write transaction commits.");
+
+ private final LongAdderMetric roCommits = new LongAdderMetric(
+ "RoCommits",
+ "Total number of read-only transaction commits.");
+
+ private final LongAdderMetric rwRollbacks = new LongAdderMetric(
+ "RwRollbacks",
+ "Total number of rolled-back read-write transactions.");
Review Comment:
I would say that it still makes sense. First of all, it is about API symmetry. Read-write transactions have commit and rollback metrics, so why should we exclude them for read-only transactions? For example, a long-running scan that was rolled back due to a timeout.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2204221095
##########
modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/ItTransactionMetricsTest.java:
##########
@@ -0,0 +1,256 @@
+/*
+ * 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.ignite.internal.tx;
+
+import static org.apache.ignite.internal.AssignmentsTestUtils.awaitAssignmentsStabilizationOnDefaultZone;
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.metrics.LongMetric;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Tests transaction metrics.
+ */
+public class ItTransactionMetricsTest extends ClusterPerClassIntegrationTest {
+ public static final String TABLE_NAME = "test_table_name";
+
+ @Override
+ protected int initialNodes() {
+ return 2;
+ }
+
+ @BeforeAll
+ void createTable() throws Exception {
+ sql("CREATE TABLE " + TABLE_NAME + " (id INT PRIMARY KEY, val VARCHAR)");
+
+ awaitAssignmentsStabilizationOnDefaultZone(CLUSTER.aliveNode());
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param nodeIndex Node index to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(int nodeIndex) {
+ return keyValueView(CLUSTER.node(nodeIndex));
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param node Node to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(Ignite node) {
+ return node.tables().table(TABLE_NAME).keyValueView(Integer.class, String.class);
+ }
+
+ /**
+ * Returns a snapshot of transaction metrics for the given node.
+ *
+ * @param nodeIndex Node index to capture transaction metrics.
+ * @return Snapshot of transaction metrics.
+ */
+ private Map<String, Long> metricValues(int nodeIndex) {
+ var values = new HashMap<String, Long>();
+
+ MetricSet txMetrics = unwrapIgniteImpl(node(nodeIndex))
+ .metricManager()
+ .metricSnapshot()
+ .metrics()
+ .get(TransactionMetricsSource.SOURCE_NAME);
+
+ txMetrics.iterator().forEachRemaining(metric -> {
+ if (metric instanceof LongMetric) {
+ values.put(metric.name(), ((LongMetric) metric).value());
+ }
+ });
+
+ return values;
+ }
+
+ private static void testMetricValues(Map<String, Long> initial, Map<String, Long> actual, String... ignored) {
+ assertThat("Number of metrics should be the same.", initial.size(), is(actual.size()));
+
+ var exclude = Set.of(ignored);
+
+ for (Map.Entry<String, Long> e : initial.entrySet()) {
+ if (!exclude.contains(e.getKey())) {
+ assertThat("Metric name = " + e.getKey(), actual.get(e.getKey()), is(e.getValue()));
+ }
+ }
+ }
+
+ /**
+ * Tests that TotalCommits and RoCommits are incremented when a read only transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadOnlyTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(tx, 12);
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits are incremented when a read write transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadWriteTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin();
+ keyValueView(0).put(tx, 12, "value");
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RwCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits/RoCommits are incremented when an "empty" transaction successfully committed.
+ * Empty means that there are no entries enlisted into the transaction.
+ */
+ @Test
+ void testCommitEmptyTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ rwTx.commit();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ roTx.commit();
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits, RwCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 2));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalRollbacks and RwRollbacks/RoRollbacks are incremented when a transaction rolled back.
+ */
+ @Test
+ void testRollbackTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ keyValueView(0).put(rwTx, 12, "value");
+ rwTx.rollback();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(roTx, 12);
+ roTx.rollback();
Review Comment:
Thank you for the suggestion. Added `testTimeoutRollbackTransaction` and `testDeadlockTransaction`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2207336542
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -712,6 +717,8 @@ public CompletableFuture<Void> finish(
)
).thenAccept(unused -> {
if (localNodeId.equals(finishingStateMeta.txCoordinatorId())) {
+ txMetrics.onReadWriteTransactionFinished(txId, commitIntent);
Review Comment:
Nice ctach! This needs to be fixed!
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2205113798
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/metrics/TransactionMetricsSource.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.ignite.internal.tx.metrics;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.ignite.internal.tx.TransactionIds.beginTimestamp;
+
+import java.util.List;
+import java.util.UUID;
+import org.apache.ignite.internal.hlc.ClockService;
+import org.apache.ignite.internal.metrics.AbstractMetricSource;
+import org.apache.ignite.internal.metrics.DistributionMetric;
+import org.apache.ignite.internal.metrics.LongAdderMetric;
+import org.apache.ignite.internal.metrics.Metric;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource.Holder;
+import org.jetbrains.annotations.TestOnly;
+
+/**
+ * Transaction metric source, that contains a set of transaction metrics.
+ **/
+public class TransactionMetricsSource extends AbstractMetricSource<Holder> {
+ /** Histogram buckets for duration metrics in milliseconds. */
+ private static final long[] HISTOGRAM_BUCKETS = new long[] {
+ MILLISECONDS.convert(1, MILLISECONDS),
+ MILLISECONDS.convert(10, MILLISECONDS),
+ MILLISECONDS.convert(100, MILLISECONDS),
+ MILLISECONDS.convert(250, MILLISECONDS),
+ MILLISECONDS.convert(500, MILLISECONDS),
+ MILLISECONDS.convert(1, SECONDS),
+ MILLISECONDS.convert(5, SECONDS)
Review Comment:
is 5 seconds enough?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -712,6 +717,8 @@ public CompletableFuture<Void> finish(
)
).thenAccept(unused -> {
if (localNodeId.equals(finishingStateMeta.txCoordinatorId())) {
+ txMetrics.onReadWriteTransactionFinished(txId, commitIntent);
Review Comment:
The `commitIntent` is the same as actual result only if the tx was really committed. It may be not true, for example, if `TxManagerImpl#verifyCommitTimestamp` threw an exception. In this case we go to `thenAccept` and `commitIntent` is correct here, but I would suggest adding `exceptionally` block with `onReadWriteTransactionFinished(txId, false)`
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/metrics/TransactionMetricsSource.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.ignite.internal.tx.metrics;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.ignite.internal.tx.TransactionIds.beginTimestamp;
+
+import java.util.List;
+import java.util.UUID;
+import org.apache.ignite.internal.hlc.ClockService;
+import org.apache.ignite.internal.metrics.AbstractMetricSource;
+import org.apache.ignite.internal.metrics.DistributionMetric;
+import org.apache.ignite.internal.metrics.LongAdderMetric;
+import org.apache.ignite.internal.metrics.Metric;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource.Holder;
+import org.jetbrains.annotations.TestOnly;
+
+/**
+ * Transaction metric source, that contains a set of transaction metrics.
+ **/
+public class TransactionMetricsSource extends AbstractMetricSource<Holder> {
+ /** Histogram buckets for duration metrics in milliseconds. */
+ private static final long[] HISTOGRAM_BUCKETS = new long[] {
+ MILLISECONDS.convert(1, MILLISECONDS),
+ MILLISECONDS.convert(10, MILLISECONDS),
+ MILLISECONDS.convert(100, MILLISECONDS),
+ MILLISECONDS.convert(250, MILLISECONDS),
+ MILLISECONDS.convert(500, MILLISECONDS),
+ MILLISECONDS.convert(1, SECONDS),
+ MILLISECONDS.convert(5, SECONDS)
+ };
+
+ /** Source name. */
+ public static final String SOURCE_NAME = "transactions";
+
+ /** Clock service to calculate a timestamp for rolled back transactions. */
+ private final ClockService clockService;
+
+ /**
+ * Creates a new instance of {@link TransactionMetricsSource}.
+ */
+ public TransactionMetricsSource(ClockService clockService) {
+ super(SOURCE_NAME, "Transaction metrics.");
+
+ this.clockService = clockService;
+ }
+
+ /**
+ * Updates read-write related metrics.
+ *
+ * @param transactionId Transaction identifier.
+ * @param commit {@code true} if a transaction was committed, and {@code false} otherwise.
+ */
+ public void onReadWriteTransactionFinished(UUID transactionId, boolean commit) {
+ Holder holder = holder();
+
+ if (holder != null) {
+ holder.rwDuration.add(calculateTransactionDuration(transactionId));
+
+ holder.activeTransactions.decrement();
+
+ if (commit) {
+ holder.totalCommits.increment();
+ holder.rwCommits.increment();
+ } else {
+ holder.totalRollbacks.increment();
+ holder.rwRollbacks.increment();
+ }
+ }
+ }
+
+ /**
+ * Updates read-only related metrics.
+ *
+ * @param transactionId Transaction identifier.
+ * @param commit {@code true} if a transaction was committed, and {@code false} otherwise.
+ */
+ public void onReadOnlyTransactionFinished(UUID transactionId, boolean commit) {
+ Holder holder = holder();
+
+ if (holder != null) {
+ holder.roDuration.add(calculateTransactionDuration(transactionId));
+
+ holder.activeTransactions.decrement();
+
+ if (commit) {
+ holder.totalCommits.increment();
+ holder.roCommits.increment();
+ } else {
+ holder.totalRollbacks.increment();
+ holder.roRollbacks.increment();
+ }
+ }
+ }
+
+ /**
+ * Tracks a number of active transactions.
+ */
+ public void onTransactionStarted() {
+ Holder holder = holder();
+
+ if (holder != null) {
+ holder.activeTransactions.increment();
+ }
+ }
+
+ /**
+ * Returns a number of active transactions.
+ * If this metric source is not enabled, then always returns {@code 0}.
+ *
+ * @return Number of active transactions.
+ */
+ public long activeTransactions() {
+ Holder holder = holder();
+
+ if (holder != null) {
+ return holder.activeTransactions.value();
+ }
+
+ return 0L;
+ }
+
+ /**
+ * Returns a number of finished transactions.
+ * If this metric source is not enabled, then always returns {@code 0}.
+ *
+ * @return Number of finished transactions.
+ */
+ public long finishedTransactions() {
+ Holder holder = holder();
+
+ if (holder != null) {
+ return holder.totalCommits.value() + holder.totalRollbacks.value();
+ }
+
+ return 0L;
+ }
+
+ @Override
+ protected Holder createHolder() {
+ return new Holder();
+ }
+
+ private long calculateTransactionDuration(UUID transactionId) {
+ return clockService.currentLong() - beginTimestamp(transactionId).getPhysical();
+ }
+
+ /** Holder. */
+ protected static class Holder implements AbstractMetricSource.Holder<Holder> {
+ private final LongAdderMetric totalCommits = new LongAdderMetric(
+ "TotalCommits",
+ "Total number of commits.");
+
+ private final LongAdderMetric totalRollbacks = new LongAdderMetric(
+ "TotalRollbacks",
+ "Total number of rollbacks.");
+
+ private final LongAdderMetric rwCommits = new LongAdderMetric(
+ "RwCommits",
+ "Total number of read-write transaction commits.");
+
+ private final LongAdderMetric roCommits = new LongAdderMetric(
+ "RoCommits",
+ "Total number of read-only transaction commits.");
+
+ private final LongAdderMetric rwRollbacks = new LongAdderMetric(
+ "RwRollbacks",
+ "Total number of rolled-back read-write transactions.");
Review Comment:
I don't think it makes sense to distinguish RO commits from RO rollbacks, it's always RO finish
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2200896632
##########
modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/ItTransactionMetricsTest.java:
##########
@@ -0,0 +1,256 @@
+/*
+ * 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.ignite.internal.tx;
+
+import static org.apache.ignite.internal.AssignmentsTestUtils.awaitAssignmentsStabilizationOnDefaultZone;
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.metrics.LongMetric;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Tests transaction metrics.
+ */
+public class ItTransactionMetricsTest extends ClusterPerClassIntegrationTest {
+ public static final String TABLE_NAME = "test_table_name";
+
+ @Override
+ protected int initialNodes() {
+ return 2;
+ }
+
+ @BeforeAll
+ void createTable() throws Exception {
+ sql("CREATE TABLE " + TABLE_NAME + " (id INT PRIMARY KEY, val VARCHAR)");
+
+ awaitAssignmentsStabilizationOnDefaultZone(CLUSTER.aliveNode());
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param nodeIndex Node index to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(int nodeIndex) {
+ return keyValueView(CLUSTER.node(nodeIndex));
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param node Node to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(Ignite node) {
+ return node.tables().table(TABLE_NAME).keyValueView(Integer.class, String.class);
+ }
+
+ /**
+ * Returns a snapshot of transaction metrics for the given node.
+ *
+ * @param nodeIndex Node index to capture transaction metrics.
+ * @return Snapshot of transaction metrics.
+ */
+ private Map<String, Long> metricValues(int nodeIndex) {
+ var values = new HashMap<String, Long>();
+
+ MetricSet txMetrics = unwrapIgniteImpl(node(nodeIndex))
+ .metricManager()
+ .metricSnapshot()
+ .metrics()
+ .get(TransactionMetricsSource.SOURCE_NAME);
+
+ txMetrics.iterator().forEachRemaining(metric -> {
+ if (metric instanceof LongMetric) {
+ values.put(metric.name(), ((LongMetric) metric).value());
+ }
+ });
+
+ return values;
+ }
+
+ private static void testMetricValues(Map<String, Long> initial, Map<String, Long> actual, String... ignored) {
+ assertThat("Number of metrics should be the same.", initial.size(), is(actual.size()));
+
+ var exclude = Set.of(ignored);
+
+ for (Map.Entry<String, Long> e : initial.entrySet()) {
+ if (!exclude.contains(e.getKey())) {
+ assertThat("Metric name = " + e.getKey(), actual.get(e.getKey()), is(e.getValue()));
+ }
+ }
+ }
+
+ /**
+ * Tests that TotalCommits and RoCommits are incremented when a read only transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadOnlyTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(tx, 12);
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits are incremented when a read write transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadWriteTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin();
+ keyValueView(0).put(tx, 12, "value");
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RwCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits/RoCommits are incremented when an "empty" transaction successfully committed.
+ * Empty means that there are no entries enlisted into the transaction.
+ */
+ @Test
+ void testCommitEmptyTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ rwTx.commit();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ roTx.commit();
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits, RwCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 2));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalRollbacks and RwRollbacks/RoRollbacks are incremented when a transaction rolled back.
+ */
+ @Test
+ void testRollbackTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ keyValueView(0).put(rwTx, 12, "value");
+ rwTx.rollback();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(roTx, 12);
+ roTx.rollback();
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalRollbacks, RwRollbacks and RoRollbacks.
+ testMetricValues(metrics0, actualMetrics0, "TotalRollbacks", "RwRollbacks", "RoRollbacks");
+
+ assertThat(actualMetrics0.get("TotalRollbacks"), is(metrics0.get("TotalRollbacks") + 2));
+ assertThat(actualMetrics0.get("RwRollbacks"), is(metrics0.get("RwRollbacks") + 1));
+ assertThat(actualMetrics0.get("RoRollbacks"), is(metrics0.get("RoRollbacks") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RoCommits are incremented when a SQL engine is used.
+ */
+ @Test
+ void testSqlImplicitTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ sql(0, "select * from " + TABLE_NAME);
Review Comment:
What about RW metric and thus RW sql query?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadOnlyTransactionImpl.java:
##########
@@ -135,7 +136,12 @@ public CompletableFuture<Void> rollbackTimeoutExceededAsync() {
}
@Override
- public CompletableFuture<Void> finish(boolean commit, HybridTimestamp executionTimestamp, boolean full, boolean timeoutExceeded) {
+ public CompletableFuture<Void> finish(
+ boolean commit,
+ @Nullable HybridTimestamp executionTimestamp,
Review Comment:
In what cases do we propagate null `executionTimestamp` for RO transactions?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java:
##########
@@ -173,15 +173,15 @@ void finishFull(
* @param timestampTracker Observable timestamp tracker is used to determine the read timestamp for read-only transactions. Each client
* should pass its own tracker to provide linearizability between read-write and read-only transactions started by this client.
* @param commitPartition Partition to store a transaction state. {@code null} if nothing was enlisted into the transaction.
- * @param commit {@code true} if a commit requested.
+ * @param commitIntent {@code true} if a commit requested.
Review Comment:
It's just a rename? You think this makes the method clearer?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -1148,11 +1158,16 @@ public CompletableFuture<Void> executeWriteIntentSwitchAsync(Runnable runnable)
return runAsync(runnable, writeIntentSwitchPool);
}
- void completeReadOnlyTransactionFuture(TxIdAndTimestamp txIdAndTimestamp, boolean timeoutExceeded) {
- finishedTxs.add(1);
-
+ void completeReadOnlyTransactionFuture(
+ boolean commit,
+ boolean implicit,
Review Comment:
Is it used?
##########
modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/ItTransactionMetricsTest.java:
##########
@@ -0,0 +1,256 @@
+/*
+ * 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.ignite.internal.tx;
+
+import static org.apache.ignite.internal.AssignmentsTestUtils.awaitAssignmentsStabilizationOnDefaultZone;
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.metrics.LongMetric;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Tests transaction metrics.
+ */
+public class ItTransactionMetricsTest extends ClusterPerClassIntegrationTest {
+ public static final String TABLE_NAME = "test_table_name";
+
+ @Override
+ protected int initialNodes() {
+ return 2;
+ }
+
+ @BeforeAll
+ void createTable() throws Exception {
+ sql("CREATE TABLE " + TABLE_NAME + " (id INT PRIMARY KEY, val VARCHAR)");
+
+ awaitAssignmentsStabilizationOnDefaultZone(CLUSTER.aliveNode());
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param nodeIndex Node index to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(int nodeIndex) {
+ return keyValueView(CLUSTER.node(nodeIndex));
+ }
+
+ /**
+ * Returns a key value view for the table {@link #TABLE_NAME}.
+ *
+ * @param node Node to create a key value view.
+ * @return Key value view.
+ */
+ private KeyValueView<Integer, String> keyValueView(Ignite node) {
+ return node.tables().table(TABLE_NAME).keyValueView(Integer.class, String.class);
+ }
+
+ /**
+ * Returns a snapshot of transaction metrics for the given node.
+ *
+ * @param nodeIndex Node index to capture transaction metrics.
+ * @return Snapshot of transaction metrics.
+ */
+ private Map<String, Long> metricValues(int nodeIndex) {
+ var values = new HashMap<String, Long>();
+
+ MetricSet txMetrics = unwrapIgniteImpl(node(nodeIndex))
+ .metricManager()
+ .metricSnapshot()
+ .metrics()
+ .get(TransactionMetricsSource.SOURCE_NAME);
+
+ txMetrics.iterator().forEachRemaining(metric -> {
+ if (metric instanceof LongMetric) {
+ values.put(metric.name(), ((LongMetric) metric).value());
+ }
+ });
+
+ return values;
+ }
+
+ private static void testMetricValues(Map<String, Long> initial, Map<String, Long> actual, String... ignored) {
+ assertThat("Number of metrics should be the same.", initial.size(), is(actual.size()));
+
+ var exclude = Set.of(ignored);
+
+ for (Map.Entry<String, Long> e : initial.entrySet()) {
+ if (!exclude.contains(e.getKey())) {
+ assertThat("Metric name = " + e.getKey(), actual.get(e.getKey()), is(e.getValue()));
+ }
+ }
+ }
+
+ /**
+ * Tests that TotalCommits and RoCommits are incremented when a read only transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadOnlyTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(tx, 12);
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits are incremented when a read write transaction successfully committed.
+ *
+ * @param implicit {@code true} if a transaction should be implicit and {@code false} otherwise.
+ */
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testCommitReadWriteTransaction(boolean implicit) {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction tx = implicit ? null : node(0).transactions().begin();
+ keyValueView(0).put(tx, 12, "value");
+ if (!implicit) {
+ tx.commit();
+ }
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits and RwCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 1));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalCommits and RwCommits/RoCommits are incremented when an "empty" transaction successfully committed.
+ * Empty means that there are no entries enlisted into the transaction.
+ */
+ @Test
+ void testCommitEmptyTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ rwTx.commit();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ roTx.commit();
+
+ Map<String, Long> actualMetrics0 = metricValues(0);
+ Map<String, Long> actualMetrics1 = metricValues(1);
+
+ // Check that there are no updates on the node 1.
+ testMetricValues(metrics1, actualMetrics1);
+
+ // Check that all transaction metrics ere not changed except TotalCommits, RwCommits and RoCommits.
+ testMetricValues(metrics0, actualMetrics0, "TotalCommits", "RwCommits", "RoCommits");
+
+ assertThat(actualMetrics0.get("TotalCommits"), is(metrics0.get("TotalCommits") + 2));
+ assertThat(actualMetrics0.get("RwCommits"), is(metrics0.get("RwCommits") + 1));
+ assertThat(actualMetrics0.get("RoCommits"), is(metrics0.get("RoCommits") + 1));
+ }
+
+ /**
+ * Tests that TotalRollbacks and RwRollbacks/RoRollbacks are incremented when a transaction rolled back.
+ */
+ @Test
+ void testRollbackTransaction() {
+ Map<String, Long> metrics0 = metricValues(0);
+ Map<String, Long> metrics1 = metricValues(1);
+
+ Transaction rwTx = node(0).transactions().begin();
+ keyValueView(0).put(rwTx, 12, "value");
+ rwTx.rollback();
+
+ Transaction roTx = node(0).transactions().begin(new TransactionOptions().readOnly(true));
+ keyValueView(0).get(roTx, 12);
+ roTx.rollback();
Review Comment:
I'd also check that total RW/RO are incremented in case of rollback caused by exception and timeout.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2207328042
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/metrics/TransactionMetricsSource.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.ignite.internal.tx.metrics;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.ignite.internal.tx.TransactionIds.beginTimestamp;
+
+import java.util.List;
+import java.util.UUID;
+import org.apache.ignite.internal.hlc.ClockService;
+import org.apache.ignite.internal.metrics.AbstractMetricSource;
+import org.apache.ignite.internal.metrics.DistributionMetric;
+import org.apache.ignite.internal.metrics.LongAdderMetric;
+import org.apache.ignite.internal.metrics.Metric;
+import org.apache.ignite.internal.tx.metrics.TransactionMetricsSource.Holder;
+import org.jetbrains.annotations.TestOnly;
+
+/**
+ * Transaction metric source, that contains a set of transaction metrics.
+ **/
+public class TransactionMetricsSource extends AbstractMetricSource<Holder> {
+ /** Histogram buckets for duration metrics in milliseconds. */
+ private static final long[] HISTOGRAM_BUCKETS = new long[] {
+ MILLISECONDS.convert(1, MILLISECONDS),
+ MILLISECONDS.convert(10, MILLISECONDS),
+ MILLISECONDS.convert(100, MILLISECONDS),
+ MILLISECONDS.convert(250, MILLISECONDS),
+ MILLISECONDS.convert(500, MILLISECONDS),
+ MILLISECONDS.convert(1, SECONDS),
+ MILLISECONDS.convert(5, SECONDS)
Review Comment:
Well, imho OLTP should be fast enough :)
We can use the same buckets as Apache Ignite 2, [METRIC_TIME_BUCKETS](https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/TransactionMetricsAdapter.java#L67)
WDYT?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2205046658
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadOnlyTransactionImpl.java:
##########
@@ -135,7 +136,12 @@ public CompletableFuture<Void> rollbackTimeoutExceededAsync() {
}
@Override
- public CompletableFuture<Void> finish(boolean commit, HybridTimestamp executionTimestamp, boolean full, boolean timeoutExceeded) {
+ public CompletableFuture<Void> finish(
+ boolean commit,
+ @Nullable HybridTimestamp executionTimestamp,
Review Comment:
BTW, I like one at a line more. Up to you.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2204212433
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java:
##########
@@ -173,15 +173,15 @@ void finishFull(
* @param timestampTracker Observable timestamp tracker is used to determine the read timestamp for read-only transactions. Each client
* should pass its own tracker to provide linearizability between read-write and read-only transactions started by this client.
* @param commitPartition Partition to store a transaction state. {@code null} if nothing was enlisted into the transaction.
- * @param commit {@code true} if a commit requested.
+ * @param commitIntent {@code true} if a commit requested.
Review Comment:
Yes, it just a rename. We discuss that with Denis Chudov in private and he mentioned that it would be nice to rename this parameter. Please take a look at [TxManagerImpl](https://github.com/apache/ignite-3/blob/main/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java#L638)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25844 Add transactions metric source [ignite-3]
Posted by "sk0x50 (via GitHub)" <gi...@apache.org>.
sk0x50 commented on code in PR #6206:
URL: https://github.com/apache/ignite-3/pull/6206#discussion_r2207322019
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadOnlyTransactionImpl.java:
##########
@@ -135,7 +136,12 @@ public CompletableFuture<Void> rollbackTimeoutExceededAsync() {
}
@Override
- public CompletableFuture<Void> finish(boolean commit, HybridTimestamp executionTimestamp, boolean full, boolean timeoutExceeded) {
+ public CompletableFuture<Void> finish(
+ boolean commit,
+ @Nullable HybridTimestamp executionTimestamp,
Review Comment:
Fixed
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadOnlyTransactionImpl.java:
##########
@@ -147,7 +147,7 @@ public CompletableFuture<Void> finish(boolean commit, HybridTimestamp executionT
txFuture.complete(null);
- ((TxManagerImpl) txManager).completeReadOnlyTransactionFuture(new TxIdAndTimestamp(readTimestamp, id()), timeoutExceeded);
+ ((TxManagerImpl) txManager).completeReadOnlyTransactionFuture(commit, new TxIdAndTimestamp(readTimestamp, id()), timeoutExceeded);
Review Comment:
:ok_hand:
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org