Enable setting of transaction expiration handler in active transaction management #1395
Conversation
| }); | ||
| } | ||
|
|
||
| public void setTransactionExpirationHandler(BiConsumer<String, DistributedTransaction> handler) { |
There was a problem hiding this comment.
We will be able to set a transaction expiration handler after this change.
There was a problem hiding this comment.
@brfrn169 Can you tell me how the custom handler would be like?
There was a problem hiding this comment.
I'm going to use it for the ScalarDB Auth thing. Theoretically, we can do anything for expired transactions with this handler. For example, we can output detail logs for the expired transactions.
| }); | ||
| } | ||
|
|
||
| public void setTransactionExpirationHandler( |
| @@ -32,6 +34,17 @@ public abstract class ActiveTransactionManagedDistributedTransactionManager | |||
|
|
|||
| private final ActiveExpiringMap<String, ActiveTransaction> activeTransactions; | |||
|
|
|||
| private final AtomicReference<BiConsumer<String, DistributedTransaction>> | |||
There was a problem hiding this comment.
Just a question.
I think we also have this expiration handler in ScalarDB Cluster, but I'm wondering if we need to have it in both sides: ScalarDB core and ScalarDB Cluster.
If the core implements it, the one in ScalarDB Cluster is not necessary?
There was a problem hiding this comment.
Sorry, I can't remember the expiration handler in ScalarDB Cluster. Please give me some more information about that?
| new AtomicReference<>( | ||
| (id, t) -> { | ||
| try { | ||
| t.rollback(); |
There was a problem hiding this comment.
[minor] I think this rollback is always necessary and any custom expiration handler must call it. But users of setTransactionExpirationHandler() can pass a custom expiration handler that doesn't call rollback by mistake. Another option would be to add a callback like this?
private final AtomicReference<BiConsumer<String, DistributedTransaction>> transactionExpirationCallback = new AtomicReference<>((id, t) -> {});
private final BiConsumer<String, DistributedTransaction>
transactionExpirationHandler =
(id, t) -> {
try {
transactionExpirationCallback.get().accept(id, t);
}
catch (Throwable t) {
// Warning
}
try {
t.rollback();
} catch (Exception e) {
logger.warn("Rollback failed. transaction ID: {}", id, e);
}
};There was a problem hiding this comment.
Good point. I agree that rollback is always necessary, which is why the default handler executes rollback. This feature is advanced and is primarily intended for internal use. Additionally, I don't plan to document it. Therefore, if a user sets a custom handler that doesn't rollback the expired transaction, I believe it falls under the user's responsibility. Thanks.
…andler-in-active-transaction-managenent
brfrn169
deleted the
enable-setting-of-transaction-expiration-handler-in-active-transaction-managenent
branch
Description
This PR enables the setting of the transaction expiration handler in active transaction management.
Related issues and/or PRs
N/A
Changes made
ActiveTransactionManagedDistributedTransactionManagerandActiveTransactionManagedTwoPhaseCommitTransactionManagerChecklist
Additional notes (optional)
N/A
Release notes
N/A