InvisibleProgrammer commented on PR #4252: URL: https://github.com/apache/hive/pull/4252#issuecomment-1525221786
@pvary , let me reflect on your writings one by one: > [* 333227fbd13821365cec1bdbfcb9314a239bea0f](c3232b664745ebf761b6a74f4c5b55cc48bfd209: Hive: Use EnvironmentContext instead of Hive Locks to provide transactional commits after HIVE-26882 - This is based on 333227fbd13821365cec1bdbfcb9314a239bea0f and fede493d59f17ff2bfc0744b296d90bd36130386. Has to be a parallel change on Hive/Impala and every writers of the Iceberg table, but fixes stability and enhances commit performance)c3232b664745ebf761b6a74f4c5b55cc48bfd209: Hive: Use EnvironmentContext instead of Hive Locks to provide transactional commits after [HIVE-26882](https://issues.apache.org/jira/browse/HIVE-26882) - This is based on 333227fbd13821365cec1bdbfcb9314a239bea0f and fede493d59f17ff2bfc0744b296d90bd36130386. Has to be a parallel change on Hive/Impala and every writers of the Iceberg table, but fixes stability and enhances commit performance That is a pretty cool change. I'm pretty sure it is worth porting. But I'm not sure if we have to port it during the 1.2.1 upgrade. What if I create a ticket to port it after we finish the 1.2.1 upgrade? > 333227fbd13821365cec1bdbfcb9314a239bea0f - Hive: Refactor commit lock mechanism from HiveTableOperations. This is mostly a refactoring to make it possible to do c3232b664745ebf761b6a74f4c5b55cc48bfd209 That looks trivial, it is easy to port it during this update. Thank your for the context. > fede493d59f17ff2bfc0744b296d90bd36130386 - Hive: Lock hardening (#6451) - makes sure that the Lock used by the Iceberg commit are cleared up... If you do not have stability issues with stuck Hive Locks then you might skip backporting it. That is the tricky and ugly one that concerns me the most: The `HiveTableOperations` class is almost 1000 lines long in the Hive repository and about 500 lines in the Iceberg one. And it looks like that commit is the root cause of the difference. But as you wrote it should be released together with all writers, it makes it not only ugly but evil as well. What do you think, even if we have no stability issues with Hive Locks, is that worth porting? What do you think, what would be the best way to handle it? My first thought is the same as at 333227fbd13821365cec1bdbfcb9314a239bea0f: we should port it in a separated ticket after the 1.2.1 upgrade. But I don't want to keep that significant difference between the two repositories for a long time. And also, do you have any suggestion about how it should be handled in the community? I mean, I assume we have to start a conversation that includes Impala and other components as well. I'm not even know, do you know how many project can be affected? -- 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]
