ctubbsii commented on a change in pull request #2467:
URL: https://github.com/apache/accumulo/pull/2467#discussion_r801174267
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
##########
@@ -136,7 +140,29 @@ public int execute(final String fullCommand, final
CommandLine cl, final Shell s
getZooReaderWriter(context, siteConfig,
cl.getOptionValue(secretOption.getOpt()));
ZooStore<FateCommand> zs = new ZooStore<>(path, zk);
- if ("fail".equals(cmd)) {
+ if ("fail-live".equals(cmd)) {
Review comment:
@dlmarion wrote:
> So, we give users the ability to create fate transactions easily.
I don't think that's an accurate characterization. Users don't create fate
transactions at all. The Manager does so to service some user requests, like
"create a table". This distinction matters, because fate transactions are an
implementation detail and should largely be hidden from the user. In many
cases, these operations are expected to be so fast (like creating a table), the
user shouldn't even have the concept of it being a queued action. The only
reason we run them as a fate operation is so we don't leave Accumulo in a dirty
state in the case of a failure. In the create table example, users don't need
to be able to cancel the fate operations, because they can just delete an
unwanted table.
So, in a lot of cases, the fate operations aren't actually there for the
user. They are there for Accumulo's internal needs. Implementation detail. So,
it makes a kind of sense that users don't directly cancel them, or can't, under
normal circumstances.
> I think in only one case, we give them the ability to cancel them (cancel
compactions).
The reason users can easily cancel compaction is because the semantics for
the compaction API is a "queue this compaction" request, not an "execute this
compaction immediately" request. The cancellation for compactions is available
to users because of the semantics of the compaction API, not because of the
implementation detail of it using fate to execute it. Those queuing and
cancelling semantics aren't generalizable to all actions that we perform via
fate and should be considered on a case-by-case basis and exposed through our
normal APIs, just like compactions are today. Interacting with Fate is not
going through normal APIs. It's surgery, and should be rare.
> Taking the manager down to fail a fate transaction is very disruptive (and
doesn't it terminate the FaTE transaction runners in the middle of their
operations?).
Aborting transaction runners in the middle of an operation is generally safe
because FaTE RepOs are designed to be idempotent and replayable to resume a
transaction. They also have the ability to roll back when they are move into a
failed state. The reason we kill the manager is to ensure we know the current
state of any transactions that are in progress that we intend to fail, so there
aren't any race conditions or risks of some side-effect of a poorly behaved
interrupted thread. That's generally acceptable because aborting fate
operations is supposed to be rare, and the surgery should be relatively safe,
with predictable outcomes. But, it's still surgery, and we don't exactly want
to lower the barrier to entry to surgery.
Fate is not intended to be a general request queue that users add and remove
stuff from at will, they are for internal consistency. When we fail stuff, it's
because we're trying to force something that isn't normal. If we want some
operations to be cancel-able, I think maybe creating a dedicated asynchronous
API for the operations we want to be able to cancel might be a better approach,
than trying to lower the barrier to entry to Fate surgery or trying to
generalize Fate operations like it's a general purpose executor service.
> Certainly the changes I made in #2462 will let users identify which
transactions have never been started, those should be able to be cancelled with
no issue.
Is the intent of this current PR to only be able to `fail-live` things that
have never been started, or operations in any state? If it's only to help
clear out unstarted things, I think I'd be more enthusiastic about it. It might
even be useful have a safe live option for `cancel all transactions not yet
started`.
--
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]
