Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Fate admin command to support the addition of the Accumulo instance type #4168

Closed
cshannon opened this issue Jan 17, 2024 · 2 comments
Assignees
Milestone

Comments

@cshannon
Copy link
Contributor

With the addition of support for multiple Fate instances and an AccumuloStore in #4133 and #4049 the fate command in the Admin utility needs to be updated.

The changes in #4133 only partially updated things to fix the problems with unit tests but more work is needed.

More investigation needs to be done for all the changes but a few things are:

  1. We need to make the command compatible with the Accumulo instance as well and not just Zookeeper for operations such as cancelling a fate operation. This will mean updating the command to provide an instance type.
  2. We should provide an option to filter transactions based on instance type.
  3. Updating the formatting of the summary output to show the instance type

We may want to wait to update the command until the changes are finished in #3964 as that might require more changes

kevinrr888 added a commit to kevinrr888/accumulo that referenced this issue Jan 30, 2024
- New fromThrift() method in FateId.
- Fate now uses FateId and resolved issues stemming from these changes.
- AdminUtil and Admin do not have FateId fully incorporated yet. Deferred for now. Marked "TODO DEFERRED - ISSUE 4044" in code. Need issue apache#4168 completed first.
- Resolved all compile errors.
@kevinrr888
Copy link
Member

I would like to work on this

kevinrr888 added a commit to kevinrr888/accumulo that referenced this issue Mar 7, 2024
Changes:
- Update summary FateOpsCommand
	- Prints full FateId not just the long
	- Option to filter based on FateInstanceType
	- Option to filter based on FateId
		- Originally, the summary command printed all Fate transactions, thought it might be good to optionally allow filtering by certain Fate transactions like print allows
- Update print FateOpsCommand
	- Prints full FateId not just the long
	- Option to filter based on FateInstanceType
	- Option to filter based on FateId
		- Originally, could filter by long id, but with the replacement of long transaction id by FateId, this had to be updated.
		- Instead of receiving "<hex id>" or "FATE[<hex id>]" on cmd line, now expects a FateId
- Update cancel, delete, and fail FateOpsCommand
	- Now work with both ZooStore and AccumuloStore by taking the full FateId on the command line and determining the store based on the FateInstanceType of the FateId
- Added tests for all 5 FateOpsCommands (FateOpsCommandsIT)
	- Tests using both AccumuloStore (AccumuloFateOpsCommandsIT) and ZooStore (ZookeeperFateOpsCommandsIT)
	- Deleted FateSummaryIT, moved the test to FateOpsCommands
@kevinrr888
Copy link
Member

This issue can be closed. Completed by #4350

@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants