ctubbsii commented on code in PR #2215:
URL: https://github.com/apache/accumulo/pull/2215#discussion_r878061657
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -478,32 +411,32 @@ public boolean prepFail(TStore<T> zs, ZooReaderWriter zk,
ServiceLockPath zLockM
try {
txid = Long.parseLong(txidStr, 16);
} catch (NumberFormatException nfe) {
- System.out.printf("Invalid transaction ID format: %s%n", txidStr);
+ log.error("Invalid transaction ID format: {}", txidStr);
return false;
}
boolean state = false;
zs.reserve(txid);
TStatus ts = zs.getStatus(txid);
switch (ts) {
case UNKNOWN:
- System.out.printf("Invalid transaction ID: %016x%n", txid);
+ log.error("Invalid transaction ID: {}", txid);
Review Comment:
These are a bit of a change in behavior for the shell, which I think
currently uses AdminUtil. I think this behavior is better, but might want to
include some release notes that mention the change, so if anybody was relying
on scripting the shell, and capturing STDOUT, it will work differently for
them, depending on their log configuration.
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java:
##########
@@ -284,6 +287,42 @@ public void waitForBalance() throws AccumuloException {
}
+ @Override
+ public void fateFail(List<String> txids) throws AccumuloException {
+ checkArgument(txids != null, "txids is null");
+ executeAdminOperation(AdminOperation.FAIL, txids, null);
+ }
+
+ @Override
+ public void fateDelete(List<String> txids) throws AccumuloException {
+ checkArgument(txids != null, "txids is null");
+ executeAdminOperation(AdminOperation.DELETE, txids, null);
+ }
+
+ @Override
+ public List<TransactionStatus> fateStatus(List<String> txids, List<String>
tStatus)
+ throws AccumuloException {
+ checkArgument(txids != null, "txids is null");
+ List<TransactionStatus> txStatus = new ArrayList<>();
+ for (var tx : executeAdminOperation(AdminOperation.PRINT, txids, tStatus))
{
+ txStatus.add(new TransactionStatus(tx.getTxid(), tx.getTstatus(),
tx.getDebug(),
+ tx.getHlocks(), tx.getWlocks(), tx.getTop(), tx.getTimecreated(),
tx.getStackInfo()));
+ }
+ return txStatus;
+ }
+
+ private List<FateTransaction> executeAdminOperation(AdminOperation op,
List<String> txids,
+ List<String> filterStatuses) throws AccumuloException {
+ try {
+ return ThriftClientTypes.CLIENT.execute(context,
+ client -> client.executeAdminOperation(TraceUtil.traceInfo(),
context.rpcCreds(), op,
+ txids, filterStatuses));
+ } catch (AccumuloSecurityException e) {
+ throw new RuntimeException("Unexpected exception thrown", e);
Review Comment:
It seems like AccumuloSecurityException should propagate up.
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -360,6 +285,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T>
zs, Set<Long> filterTx
List<Long> transactions = zs.list();
List<TransactionStatus> statuses = new ArrayList<>(transactions.size());
+ Gson gson = new GsonBuilder()
+ .registerTypeAdapter(ReadOnlyRepo.class, new InterfaceSerializer<>())
+ .registerTypeAdapter(Repo.class, new InterfaceSerializer<>())
+ .registerTypeAdapter(byte[].class, new
ByteArraySerializer()).setPrettyPrinting().create();
Review Comment:
What are these type adapters doing? An inline comment could help.
##########
server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java:
##########
@@ -447,6 +460,83 @@ public List<TDiskUsage> getDiskUsage(Set<String> tables,
TCredentials credential
}
}
+ @Override
+ public List<FateTransaction> executeAdminOperation(TInfo tInfo, TCredentials
credentials,
+ AdminOperation op, List<String> txids, List<String> filterStatuses)
+ throws ThriftSecurityException, TException {
+ try {
+ authenticate(tInfo, credentials);
Review Comment:
In addition to authenticating, we probably want to restrict this to users
with admin permissions, right?
--
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]
