keith-turner commented on a change in pull request #2467:
URL: https://github.com/apache/accumulo/pull/2467#discussion_r801036570
##########
File path: core/src/main/java/org/apache/accumulo/fate/Fate.java
##########
@@ -300,6 +331,68 @@ public TStatus waitForCompletion(long tid) {
return store.waitForStatusChange(tid, FINISHED_STATES);
}
+ /**
+ * Attempts to cancel a running Fate transaction
+ *
+ * @param tid
+ * transaction id
+ * @return true if transaction transitioned to a failed state or already in
a completed state,
+ * false otherwise
+ */
+ public boolean cancel(long tid) {
+ String tidStr = Long.toHexString(tid);
+ for (int retries = 0; retries < 5; retries++) {
+ if (store.tryReserve(tid)) {
+ try {
+ TStatus status = store.getStatus(tid);
+ if (status == NEW || status == SUBMITTED || status == IN_PROGRESS) {
+ store.setProperty(tid, EXCEPTION_PROP, new TApplicationException(
+ TApplicationException.INTERNAL_ERROR, "Fate transaction
cancelled by user"));
+ store.setStatus(tid, FAILED_IN_PROGRESS);
+ log.info(
+ "Updated status for Repo {} to FAILED_IN_PROGRESS because it
was cancelled by user",
+ tidStr);
+ return true;
+ } else {
+ log.info("Repo {} cancelled by user but already in finished
state", tidStr);
+ return true;
+ }
+ } finally {
+ store.unreserve(tid, 0);
+ }
+ } else {
+ // It's possible that the FateOp is being run by the
TransactionRunner. Look up
+ // the thread that is running this transaction.
+ Thread t = null;
+ synchronized (RUNNING_TRANSACTIONS) {
+ t = RUNNING_TRANSACTIONS.get(tid);
+ }
+ if (t != null) {
+ Thread t2 = null;
+ synchronized (t) {
+ // Make sure that the transaction is still associated
+ // with this thread. If it is, then this thread is blocked
+ // from removing this transaction from the table and we can
+ // interrupt it and cause it to fail.
+ synchronized (RUNNING_TRANSACTIONS) {
+ t2 = RUNNING_TRANSACTIONS.get(tid);
+ }
+ if (t2 != null && t == t2) {
+ t.interrupt();
+ }
+ }
+ return true;
+ } else {
+ // reserved, but not in transaction table. It should transition to
+ // an end state or become unreserved in a short amount of time.
+ // Lets retry.
+ UtilWaitThread.sleep(500);
+ }
Review comment:
```suggestion
Thread t = null;
synchronized (RUNNING_TRANSACTIONS) {
t = RUNNING_TRANSACTIONS.get(tid);
if( t != null){
//must interrupt in sync block to ensure thread is still
associated with transaction...
// however if we interrupt the thread and nothing checks the
interrupt status then
// it could still impact an unrelated FATE tx, added a TODO
about that elsewhere
t.interrupt();
return true;
}
}
// TODO not sure if I got the flow/branch on the following part
correct
// reserved, but not in transaction table. It should transition to
// an end state or become unreserved in a short amount of time.
// Lets retry.
UtilWaitThread.sleep(500);
```
##########
File path: core/src/main/java/org/apache/accumulo/fate/Fate.java
##########
@@ -71,24 +83,43 @@ public void run() {
try {
tid = store.reserve();
TStatus status = store.getStatus(tid);
+ if (status == FAILED) {
+ runnerLog.info(
+ "FATE txid " + Long.toHexString(tid) + " likely cancelled
before it started.");
+ return;
+ }
Repo<T> op = store.top(tid);
- if (status == TStatus.FAILED_IN_PROGRESS) {
+ if (status == FAILED_IN_PROGRESS) {
processFailed(tid, op);
} else {
Repo<T> prevOp = null;
try {
- deferTime = op.isReady(tid, environment);
-
- if (deferTime == 0) {
- prevOp = op;
- if (status == TStatus.SUBMITTED) {
- store.setStatus(tid, TStatus.IN_PROGRESS);
+ try {
+ synchronized (RUNNING_TRANSACTIONS) {
+ RUNNING_TRANSACTIONS.put(tid, Thread.currentThread());
}
- op = op.call(tid, environment);
- } else
- continue;
-
+ deferTime = op.isReady(tid, environment);
+ if (deferTime == 0) {
+ prevOp = op;
+ if (status == SUBMITTED) {
+ store.setStatus(tid, IN_PROGRESS);
+ }
+ op = op.call(tid, environment);
+ } else
+ continue;
+ } finally {
+ synchronized (Thread.currentThread()) {
+ synchronized (RUNNING_TRANSACTIONS) {
+ RUNNING_TRANSACTIONS.remove(tid);
+ }
+ }
Review comment:
```suggestion
synchronized (RUNNING_TRANSACTIONS) {
RUNNING_TRANSACTIONS.remove(tid);
}
// TODO maybe we should check/clear for interrupt here...
if something
// was trying to interrupt right before the sync block
then we do not want
// that to impact the next thing this thread will work
on....
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
