This is an automated email from the ASF dual-hosted git repository.
ddanielr pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 3ce9fead76 Fixes the shell fate delete command (#6051)
3ce9fead76 is described below
commit 3ce9fead761efdb4ee9f6208f456428d70451b5d
Author: Daniel Roberts <[email protected]>
AuthorDate: Tue Feb 10 16:31:50 2026 -0500
Fixes the shell fate delete command (#6051)
* Fixes the shell fate delete command
Changes the delete action of the `shell fate --delete` to actually
remove the fate transaction id from table_locks instead of looking for
the zlock under the manager lock.
This matches the same delete behavior as the `admin fate --delete` command
* fix-formatting
* Added test for fate table lock deletes
Added a test for deleting table locks from a hung fate transaction
---
.../org/apache/accumulo/core/fate/AdminUtil.java | 10 ++++-
.../org/apache/accumulo/core/fate/FateTxId.java | 13 ++++++
.../accumulo/shell/commands/FateCommand.java | 9 ++--
.../accumulo/shell/commands/FateCommandTest.java | 3 +-
.../accumulo/test/fate/zookeeper/FateIT.java | 48 ++++++++++++++++++++++
5 files changed, 76 insertions(+), 7 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/fate/AdminUtil.java
b/core/src/main/java/org/apache/accumulo/core/fate/AdminUtil.java
index 3ea322ef78..c1acd45fe2 100644
--- a/core/src/main/java/org/apache/accumulo/core/fate/AdminUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/fate/AdminUtil.java
@@ -519,7 +519,13 @@ public class AdminUtil<T> {
return state;
}
- public void deleteLocks(ZooReaderWriter zk, ServiceLock.ServiceLockPath
path, String txidStr)
+ /**
+ * Removes locks at specific serviceLockPaths
+ * <p>
+ * When removing fate table locks, the txIdStr value must be the fate txId
in hex format. See
+ * {@link FateTxId#toHexString(String)}
+ */
+ public void deleteLocks(ZooReaderWriter zk, ServiceLock.ServiceLockPath
path, String txIdStr)
throws KeeperException, InterruptedException {
// delete any locks assoc w/ fate operation
List<String> lockedIds = zk.getChildren(path.toString());
@@ -530,7 +536,7 @@ public class AdminUtil<T> {
String lockPath = path + "/" + id + "/" + node;
byte[] data = zk.getData(path + "/" + id + "/" + node);
String[] lda = new String(data, UTF_8).split(":");
- if (lda[1].equals(txidStr)) {
+ if (lda[1].equals(txIdStr)) {
zk.recursiveDelete(lockPath, NodeMissingPolicy.SKIP);
}
}
diff --git a/core/src/main/java/org/apache/accumulo/core/fate/FateTxId.java
b/core/src/main/java/org/apache/accumulo/core/fate/FateTxId.java
index 8468eda355..6c646baac9 100644
--- a/core/src/main/java/org/apache/accumulo/core/fate/FateTxId.java
+++ b/core/src/main/java/org/apache/accumulo/core/fate/FateTxId.java
@@ -22,6 +22,7 @@ import java.util.regex.Pattern;
import org.apache.accumulo.core.util.FastFormat;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
public class FateTxId {
@@ -51,6 +52,18 @@ public class FateTxId {
return Long.parseLong(getHex(fmtTid), 16);
}
+ /**
+ * Returns the hex value of the FateTxId from a formatted fate transaction
+ *
+ * @param fmtTid formatted fate transaction
+ * @return hex value of long txId
+ */
+ @VisibleForTesting
+ public static String toHexString(String fmtTid) {
+ Preconditions.checkArgument(isFormatedTid(fmtTid));
+ return getHex(fmtTid);
+ }
+
/**
* Formats transaction ids in a consistent way that is useful for logging
and persisting.
*/
diff --git
a/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
b/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
index 881b2f99a1..82f9366748 100644
--- a/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
+++ b/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
@@ -170,7 +170,8 @@ public class FateCommand extends Command {
} else if (cl.hasOption(delete.getOpt())) {
String[] txids = cl.getOptionValues(delete.getOpt());
validateArgs(txids);
- failedCommand = deleteTx(shellState.getWriter(), admin, zs, zk,
managerLockPath, txids);
+ failedCommand =
+ deleteTx(shellState.getWriter(), admin, zs, zk, managerLockPath,
tableLocksPath, txids);
} else if (cl.hasOption(list.getOpt())) {
printTx(shellState, admin, zs, zk, tableLocksPath,
cl.getOptionValues(list.getOpt()), cl);
} else if (cl.hasOption(print.getOpt())) {
@@ -237,11 +238,11 @@ public class FateCommand extends Command {
}
protected boolean deleteTx(PrintWriter out, AdminUtil<FateCommand> admin,
- ZooStore<FateCommand> zs, ZooReaderWriter zk, ServiceLockPath
zLockManagerPath, String[] args)
- throws InterruptedException, KeeperException {
+ ZooStore<FateCommand> zs, ZooReaderWriter zk, ServiceLockPath
zLockManagerPath,
+ ServiceLockPath zTableLocksPath, String[] args) throws
InterruptedException, KeeperException {
for (int i = 1; i < args.length; i++) {
if (admin.prepDelete(zs, zk, zLockManagerPath, args[i])) {
- admin.deleteLocks(zk, zLockManagerPath, args[i]);
+ admin.deleteLocks(zk, zTableLocksPath, args[i]);
} else {
out.printf("Could not delete transaction: %s%n", args[i]);
return false;
diff --git
a/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
b/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
index 2419cbf9d8..fc02a83d82 100644
---
a/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
+++
b/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
@@ -100,7 +100,8 @@ public class FateCommandTest {
@Override
protected boolean deleteTx(PrintWriter out, AdminUtil<FateCommand> admin,
ZooStore<FateCommand> zs, ZooReaderWriter zk, ServiceLockPath
zLockManagerPath,
- String[] args) throws InterruptedException, KeeperException {
+ ServiceLockPath zTableLocksPath, String[] args)
+ throws InterruptedException, KeeperException {
deleteCalled = true;
return true;
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/FateIT.java
b/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/FateIT.java
index e6029d5649..0fa610c509 100644
--- a/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/FateIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/FateIT.java
@@ -51,12 +51,14 @@ import org.apache.accumulo.core.conf.ConfigurationCopy;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.NamespaceId;
import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.fate.AdminUtil;
import org.apache.accumulo.core.fate.AgeOffStore;
import org.apache.accumulo.core.fate.Fate;
import org.apache.accumulo.core.fate.FateTxId;
import org.apache.accumulo.core.fate.ReadOnlyTStore.TStatus;
import org.apache.accumulo.core.fate.Repo;
import org.apache.accumulo.core.fate.ZooStore;
+import org.apache.accumulo.core.fate.zookeeper.ServiceLock;
import org.apache.accumulo.core.fate.zookeeper.ZooCache;
import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter;
import org.apache.accumulo.core.util.UtilWaitThread;
@@ -65,6 +67,7 @@ import org.apache.accumulo.manager.tableOps.ManagerRepo;
import org.apache.accumulo.manager.tableOps.TraceRepo;
import org.apache.accumulo.manager.tableOps.Utils;
import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.util.Admin;
import org.apache.accumulo.test.util.Wait;
import org.apache.accumulo.test.zookeeper.ZooKeeperTestingServer;
import org.apache.zookeeper.KeeperException;
@@ -404,6 +407,51 @@ public class FateIT {
assertFalse(fate.cancel(txid));
}
+ /**
+ * Test that verifies that fate table locks are created and deleted from the
correct ZooKeeper
+ * path
+ */
+ @Test
+ public void testTableLockDeleteUsesCorrectZKPath() throws Exception {
+
+ ConfigurationCopy config = new ConfigurationCopy();
+ config.set(Property.GENERAL_THREADPOOL_SIZE, "2");
+ config.set(Property.MANAGER_FATE_THREADPOOL_SIZE, "1");
+ AdminUtil<Admin> admin = new AdminUtil<>(true);
+
+ callStarted = new CountDownLatch(1);
+ finishCall = new CountDownLatch(1);
+
+ long txId = fate.startTransaction();
+ String formattedTxId = FateTxId.formatTid(txId);
+ LOG.debug("Starting test testDeleteUsesCorrectZKPath {}", formattedTxId);
+ assertEquals(NEW, getTxStatus(zk, txId));
+ fate.seedTransaction("TestOperation", txId, new TestOperation(NS, TID),
false,
+ "Test Delete Op");
+ assertEquals(SUBMITTED, getTxStatus(zk, txId));
+ fate.startTransactionRunners(config, new ScheduledThreadPoolExecutor(2));
+ // Wait for the transaction runner to be in progress
+ Wait.waitFor(() -> IN_PROGRESS == getTxStatus(zk, txId));
+
+ assertFalse(fate.cancel(txId));
+
+ var tableLocksPath = ServiceLock.path(ZK_ROOT + Constants.ZTABLE_LOCKS);
+ assertFalse(zk.getChildren(tableLocksPath.toString()).isEmpty(),
+ "Table locks at " + tableLocksPath + "do not exist");
+ for (var tableName : zk.getChildren(tableLocksPath.toString())) {
+ for (var tableLock : zk.getChildren(
+ ServiceLock.path(ZK_ROOT + Constants.ZTABLE_LOCKS + "/" +
tableName).toString())) {
+ LOG.debug("Found table {} with fate lock {}", tableName, tableLock);
+ }
+ }
+
+ admin.deleteLocks(zk, tableLocksPath, FateTxId.toHexString(formattedTxId));
+ for (var tableName : zk.getChildren(tableLocksPath.toString())) {
+ var fateTableLocks = tableLocksPath + "/" + tableName;
+ assertTrue(zk.getChildren(fateTableLocks).isEmpty(), " table fate locks
are still present");
+ }
+ }
+
@Test
public void testRepoFails() throws Exception {
/*
