kevinrr888 opened a new pull request, #5028:
URL: https://github.com/apache/accumulo/pull/5028
This PR makes several improvements/fixes: improvements to the `admin fate`
util which were made possible from #4524, replaced incorrect use of
`createDummyLockID()` in real code (now only used in tests),
`<User/Meta>FateStore` now support a null lock id if they will be used as
read-only stores: write ops will fail on a store with a null lock, and some
other misc. changes.
Full list of changes:
- Removed the check for a dead Manager in the Admin fate util (AdminUtil)
which was checked before `admin fate delete <tx>` or `admin fate fail <tx>` was
able to run. This check is no longer needed with the changes from #4524. #4524
moved reservations out of Manager memory into the FATE table (for
UserFateStore) and into ZK (for MetaFateStore). Prior to this, the Admin
process would have no way of knowing if the Manager process had a transaction
reserved, so the Manager had to be shutdown to ensure it was not. But now that
reservations are visible to any process, we can try to reserve the transaction
in Admin, and if it cannot be reserved and deleted/failed in a reasonable time,
we let the user know that the Manager would need to be shutdown if
deleting/failing the transaction is still desired.
- This has several benefits:
- It is one less thing to worry about when implementing multiple
managers in the future since Admin assumes only one Manager for these commands.
However, there is still the case where the Manager may
keep a transaction reserved for a long period of time and the Admin can
never reserve it. In this case, we inform the user that the transaction could
not be deleted/failed and that if deleting/failing
is still desired, the Manager may need to be shutdown.
- It covers a potential issue in the previously existing code where
there was nothing stopping or ensuring a Manager is not started after the check
is already performed in Admin but before the delete/
fail was executed.
- It also should make the commands easier to use now since the Manager
is not required to be shutdown before use.
- Changes and adds some tests for `admin fate fail` and `admin fate delete`:
ensures the Manager is not required to be down to fail/delete a transaction,
and ensures that if the Manager does have a transaction reserved, admin will
fail to reserve and fail/delete the transaction.
- Another change which was needed as a prerequisite for the above changes
was creating a ZK lock for Admin so transactions can be properly reserved by
the command. Added new constant `Constants.ZADMIN_LOCK = "/admin/lock"`,
changed `ServiceLockPaths`, and added `Admin.createAdminLock()` to support this
- New class `TestLock` (in test package) which is used by tests to create a
real ZK lock, or a dummy one. Removed `createDummyLockID()` from
`AbstractFateStore` (moved to TestLock), and `createDummyLock()` is now only
used in test code. Added new constant `ZTEST_LOCK = "/test/lock"`, changed
`ServiceLockPaths`, and added `createTestLock()` which is used to create a real
lock id (one held in ZK) which is needed for some tests.
- This fixes an unexpected failure that could have occurred for
`ExternalCompaction_1_IT`. Was using a dummy lock for the store before and the
fate data was being stored in the same locations that the
Manager uses for it's fate data. The DeadReservationCleaner running in
Manager would have cleaned up reservations created using this store if it ran
when reservations were present. Now the test creates
a real ZK lock so the DeadReservationCleaner won't clean these up
unexpectedly.
- Stores now support a null lock id for the situation where they will be
used as read-only stores. A store with a null lock id will fail on write ops.
Changed all existing uses of stores to only have a lock id if writes will occur
(previously, all instances of the stores had a lock id).
- Removed unused or unneccesary constructors for AbstractFateStore,
MetaFateStore, UserFateStore
- Ensured all tests changed, all FATE tests, and sunny day tests still pass
closes #4904
--
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]
