Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern merged PR #4690: URL: https://github.com/apache/ignite-3/pull/4690 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1841677218
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
Review Comment:
After private discussions, the PR was reworked so that the system view now
uses existing collections.
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1841686142
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
Review Comment:
The PR was reworked so that the system view now uses existing collections.
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1841685512
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -961,11 +978,35 @@ private HybridTimestamp
createBeginTimestampWithIncrementRwTxCounter() {
});
}
+/** Called when a read-write transaction is finished. */
+private void onFinishRwTx(UUID txId) {
+decrementRwTxCount(txId);
+
+unregister(txId);
+}
+
private void decrementRwTxCount(UUID txId) {
localRwTxCounter.inUpdateRwTxCountLock(() -> {
localRwTxCounter.decrementRwTxCount(beginTimestamp(txId));
return null;
});
}
+
+/**
+ * Puts a transaction into the registry.
+ *
+ * @param tx Transaction.
+ * @return Registered transaction.
+ */
+private InternalTransaction register(InternalTransaction tx) {
+transactions.put(tx.id(), tx);
Review Comment:
The PR was reworked so that the system view now uses existing collections.
Review it again, please.
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1841684758
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
+
+private final TransactionsViewProvider txSystemViewProvider = new
TransactionsViewProvider(transactions);
Review Comment:
The PR was reworked so that the system view now uses existing collections.
But now provider requires data that is become available only after component
start
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1841684758
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
+
+private final TransactionsViewProvider txSystemViewProvider = new
TransactionsViewProvider(transactions);
Review Comment:
Thanks, the PR was reworked so that the system view now uses existing
collections. But now provider requires data that is become available only after
component start
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
korlov42 commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1836455962
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
+
+private final TransactionsViewProvider txSystemViewProvider = new
TransactionsViewProvider(transactions);
Review Comment:
do we really need `viewProvide` as a member of `TxManager`? The former is
stateless object with no particular lifecycle. Besides, system views are
supposed to be registered only once. Therefore, it would be better I think to
not pollute manager with unnecessary members (e.g. create provider and
immediately call invoke `get()` method inside `systemViews()` method).
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -961,11 +978,35 @@ private HybridTimestamp
createBeginTimestampWithIncrementRwTxCounter() {
});
}
+/** Called when a read-write transaction is finished. */
+private void onFinishRwTx(UUID txId) {
+decrementRwTxCount(txId);
+
+unregister(txId);
+}
+
private void decrementRwTxCount(UUID txId) {
localRwTxCounter.inUpdateRwTxCountLock(() -> {
localRwTxCounter.decrementRwTxCount(beginTimestamp(txId));
return null;
});
}
+
+/**
+ * Puts a transaction into the registry.
+ *
+ * @param tx Transaction.
+ * @return Registered transaction.
+ */
+private InternalTransaction register(InternalTransaction tx) {
+transactions.put(tx.id(), tx);
Review Comment:
let's add an assertion to make sure we are not overriding another
transaction here (e.g. tx is registered only once; there is no duplicate ids at
least locally)
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1836137552
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
Review Comment:
Please check my [answer
below](https://github.com/apache/ignite-3/pull/4690#discussion_r1836119021)
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
xtern commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1836119021
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
Review Comment:
@vldpyatkov , we need registry of running transactions to be able to abort
pending transaction (command `KILL TRANSACTION TxId`).
`VolatileTxStateMetaStorage` contains only states for RW transactions.
Can you suggest how the command should be designed to abort any running
transaction (including read-only transactions) without such mapping?
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
zstan commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1836091466
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
Review Comment:
It need to be filtered by TxStateMeta#txCoordinatorId
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
zstan commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1836086781
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
Review Comment:
Seems we already have tx states storage VolatileTxStateMetaStorage
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-23431 Added TRANSACTIONS system view [ignite-3]
vldpyatkov commented on code in PR #4690:
URL: https://github.com/apache/ignite-3/pull/4690#discussion_r1836087091
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##
@@ -192,6 +196,11 @@ public class TxManagerImpl implements TxManager,
NetworkMessageHandler {
private final ReplicaService replicaService;
+/** Registry of locally started active transactions. */
+private final Map transactions = new
ConcurrentHashMap<>();
Review Comment:
This map has a risk for performance. I think we can reuse another existing
storage (for example VolatileTxStateMetaStorage).
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
