pvary opened a new pull request, #6570:
URL: https://github.com/apache/iceberg/pull/6570
HIVE-26882 gives us the possibility to atomically change the
`metadata_location` using a single `alter_table` HMS call. The change is a new
feature, so exception is needed from the Hive community to include it to new
releases. OTOH I deliberately kept simple, so if someone uses their own Hive
release they could backport the change easily.
If we start using this possibility we can avoid using HMS locks to ensure
the atomicity of the `HiveTableOperations.commit`. This has the following
benefits:
- Instead of 4 HMS calls (lock, getTable, alter_table, unlock) we can use
only 2 calls (getTable, alter_table) which could help in commit performance.
- I have seen several complains when the locks were not removed correctly
(#3336, #2301) and suggested solutions (#6370). Also did my share to fix the
situation as much as possible too (#6451)
- Removing the locks would remove much of this complexity
The solution has 2 parts:
- I rebased and refreshed @szlta's work on #5877 to refactor out the Lock
related code to its own class
- Added a new feature to disable the locking mechanism
Some of my concerns and thoughts:
- I am not sure when HIVE-26882 will be available in OS. Here I would ask
help from the Hive folks: @InvisibleProgrammer, @TuroczyX, @deniskuzZ.
- The simplicity of the Hive PR helps alleviating my fears here
- We already have `LockManager` interface defined in the `iceberg-api`
module. After some back-and-forth I decided against using it, because of the
following reasons:
- I do not think anyone would like to use HiveLockManager without
HiveCatalog
- The interface is not that useful for us:
- We would need to keep track of the HMS lockId internally
- We would need to update the LockManager if the `setConf` method is
called on the HiveCatalog
- We would need to add something like `ensureActive` to the
interface which is needed for HiveTableOperations
- The `BaseLockManager` does not provide too much of a functionality
- The current configuration keys are different from the ones used by the
`LockManager` implementations
WDYT?
Open for comments and suggestions.
Thanks,
Peter
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
