[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839260=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839260 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 15/Jan/23 09:32 Start Date: 15/Jan/23 09:32 Worklog Time Spent: 10m Work Description: pvary merged PR #3947: URL: https://github.com/apache/hive/pull/3947 Issue Time Tracking --- Worklog Id: (was: 839260) Time Spent: 4h 40m (was: 4.5h) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 4h 40m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839259=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839259 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 15/Jan/23 09:31 Start Date: 15/Jan/23 09:31 Worklog Time Spent: 10m Work Description: pvary merged PR #3946: URL: https://github.com/apache/hive/pull/3946 Issue Time Tracking --- Worklog Id: (was: 839259) Time Spent: 4.5h (was: 4h 20m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 4.5h > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839257=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839257 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 15/Jan/23 08:14 Start Date: 15/Jan/23 08:14 Worklog Time Spent: 10m Work Description: pvary merged PR #3944: URL: https://github.com/apache/hive/pull/3944 Issue Time Tracking --- Worklog Id: (was: 839257) Time Spent: 4h 20m (was: 4h 10m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 4h 20m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839256=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839256 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 15/Jan/23 08:11 Start Date: 15/Jan/23 08:11 Worklog Time Spent: 10m Work Description: pvary merged PR #3943: URL: https://github.com/apache/hive/pull/3943 Issue Time Tracking --- Worklog Id: (was: 839256) Time Spent: 4h 10m (was: 4h) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 4h 10m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839249=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839249 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 14/Jan/23 20:54 Start Date: 14/Jan/23 20:54 Worklog Time Spent: 10m Work Description: pvary opened a new pull request, #3947: URL: https://github.com/apache/hive/pull/3947 This is the branch-2.3 backport of the following PR https://github.com/apache/hive/pull/3888 / [HIVE-26882](https://issues.apache.org/jira/browse/HIVE-26882): Allow transactional check of Table parameter before altering the Table Issue Time Tracking --- Worklog Id: (was: 839249) Time Spent: 4h (was: 3h 50m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 4h > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839248=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839248 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 14/Jan/23 20:53 Start Date: 14/Jan/23 20:53 Worklog Time Spent: 10m Work Description: pvary opened a new pull request, #3946: URL: https://github.com/apache/hive/pull/3946 This is the branch-2 backport of the following PR https://github.com/apache/hive/pull/3888 / [HIVE-26882](https://issues.apache.org/jira/browse/HIVE-26882): Allow transactional check of Table parameter before altering the Table Issue Time Tracking --- Worklog Id: (was: 839248) Time Spent: 3h 50m (was: 3h 40m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 3h 50m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839220=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839220 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 14/Jan/23 07:42 Start Date: 14/Jan/23 07:42 Worklog Time Spent: 10m Work Description: pvary opened a new pull request, #3944: URL: https://github.com/apache/hive/pull/3944 This is the branch-3.1 backport of the following PR https://github.com/apache/hive/pull/3888 / [HIVE-26882](https://issues.apache.org/jira/browse/HIVE-26882): Allow transactional check of Table parameter before altering the Table Issue Time Tracking --- Worklog Id: (was: 839220) Time Spent: 3h 40m (was: 3.5h) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 3h 40m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=839107=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839107 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 13/Jan/23 15:24 Start Date: 13/Jan/23 15:24 Worklog Time Spent: 10m Work Description: pvary opened a new pull request, #3943: URL: https://github.com/apache/hive/pull/3943 This is the branch-3 backport of the following PR #3888 / HIVE-26882: Allow transactional check of Table parameter before altering the Table Issue Time Tracking --- Worklog Id: (was: 839107) Time Spent: 3.5h (was: 3h 20m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 3.5h > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=83=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-83 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 08/Jan/23 22:13 Start Date: 08/Jan/23 22:13 Worklog Time Spent: 10m Work Description: pvary commented on PR #3888: URL: https://github.com/apache/hive/pull/3888#issuecomment-1374940958 Pushed to master. Thanks @prasanthj and @szehon-ho for the review! Issue Time Tracking --- Worklog Id: (was: 83) Time Spent: 3h 20m (was: 3h 10m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Time Spent: 3h 20m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837776=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837776 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 08/Jan/23 22:02 Start Date: 08/Jan/23 22:02 Worklog Time Spent: 10m Work Description: pvary merged PR #3888: URL: https://github.com/apache/hive/pull/3888 Issue Time Tracking --- Worklog Id: (was: 837776) Time Spent: 3h 10m (was: 3h) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Assignee: Peter Vary >Priority: Major > Labels: pull-request-available > Time Spent: 3h 10m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837640=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837640
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 06/Jan/23 22:54
Start Date: 06/Jan/23 22:54
Worklog Time Spent: 10m
Work Description: szehon-ho commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1063864245
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##
@@ -6038,17 +6039,26 @@ public void alter_table_with_environment_context(final
String dbname,
throws InvalidOperationException, MetaException {
String[] parsedDbName = parseDbName(dbname, conf);
alter_table_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME],
-name, newTable, envContext, null, null, null);
+name, newTable, envContext, null, null, null, null, null);
}
private void alter_table_core(String catName, String dbname, String name,
Table newTable,
-EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities, String processorId)
+EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities,
+String processorId, String
expectedPropertyKey, String expectedPropertyValue)
Review Comment:
Though I still think its cleaner to support multi-properties transactional
check, given Table has properties map, I'm leave it to you and @prasanthj
Issue Time Tracking
---
Worklog Id: (was: 837640)
Time Spent: 3h (was: 2h 50m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Assignee: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 3h
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837639=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837639
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 06/Jan/23 22:54
Start Date: 06/Jan/23 22:54
Worklog Time Spent: 10m
Work Description: szehon-ho commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1063863915
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
Sounds good.
Issue Time Tracking
---
Worklog Id: (was: 837639)
Time Spent: 2h 50m (was: 2h 40m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Assignee: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 2h 50m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837461=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837461
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 06/Jan/23 12:04
Start Date: 06/Jan/23 12:04
Worklog Time Spent: 10m
Work Description: pvary commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1063384632
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##
@@ -6038,17 +6039,26 @@ public void alter_table_with_environment_context(final
String dbname,
throws InvalidOperationException, MetaException {
String[] parsedDbName = parseDbName(dbname, conf);
alter_table_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME],
-name, newTable, envContext, null, null, null);
+name, newTable, envContext, null, null, null, null, null);
}
private void alter_table_core(String catName, String dbname, String name,
Table newTable,
-EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities, String processorId)
+EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities,
+String processorId, String
expectedPropertyKey, String expectedPropertyValue)
Review Comment:
After some more thought, I would opt for keeping the PR as it is - checking
only for a single property.
If someone would need more properties to check, then they still could create
an uber property - as a hash of the relevant properties -, and use the
`EXPECTED_PARAMETER_KEY` and `EXPECTED_PARAMETER_VALUE` to check for the
changes of this uber property. And if it is that hard for them, they could
still add a new feature later.
Issue Time Tracking
---
Worklog Id: (was: 837461)
Time Spent: 2h 40m (was: 2.5h)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 2h 40m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837413=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837413
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 06/Jan/23 06:06
Start Date: 06/Jan/23 06:06
Worklog Time Spent: 10m
Work Description: pvary commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1063146796
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
This is the relevant code part:
```
msdb.openTransaction(Constants.TX_REPEATABLE_READ)
```
Issue Time Tracking
---
Worklog Id: (was: 837413)
Time Spent: 2.5h (was: 2h 20m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 2.5h
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837412=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837412
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 06/Jan/23 06:03
Start Date: 06/Jan/23 06:03
Worklog Time Spent: 10m
Work Description: pvary commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1063145114
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
Users does not have to do anything. We set the isolation level per
transaction automatically from the code. Similarly as we do in the TxnHandler
methods where the default isolation level is not enough.
Issue Time Tracking
---
Worklog Id: (was: 837412)
Time Spent: 2h 20m (was: 2h 10m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837355=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837355
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 05/Jan/23 22:41
Start Date: 05/Jan/23 22:41
Worklog Time Spent: 10m
Work Description: szehon-ho commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1062793127
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
Thanks for checking. Is this a setting user can configure in Hive? If so,
it sounds like we should document it as a pre-requisite for user to enable this
feature
Issue Time Tracking
---
Worklog Id: (was: 837355)
Time Spent: 2h 10m (was: 2h)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837072=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837072 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 04/Jan/23 23:07 Start Date: 04/Jan/23 23:07 Worklog Time Spent: 10m Work Description: sonarcloud[bot] commented on PR #3888: URL: https://github.com/apache/hive/pull/3888#issuecomment-1371529261 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=3888) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=3888=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=3888=duplicated_lines_density=list) No Duplication information Issue Time Tracking --- Worklog Id: (was: 837072) Time Spent: 2h (was: 1h 50m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Priority: Major > Labels: pull-request-available > Time Spent: 2h > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837016=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837016
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 04/Jan/23 19:47
Start Date: 04/Jan/23 19:47
Worklog Time Spent: 10m
Work Description: pvary commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1061829069
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java:
##
@@ -1183,6 +1193,90 @@ public void testAlterTableAlreadyExists() throws
Exception {
}
}
+ @Test
+ public void testAlterTableExpectedPropertyMatch() throws Exception {
+Table originalTable = testTables[0];
+
+EnvironmentContext context = new EnvironmentContext();
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY,
"transient_lastDdlTime");
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE,
+originalTable.getParameters().get("transient_lastDdlTime"));
+
+client.alter_table(originalTable.getCatName(), originalTable.getDbName(),
originalTable.getTableName(),
+originalTable, context);
+ }
+
+ @Test(expected = MetaException.class)
+ public void testAlterTableExpectedPropertyDifferent() throws Exception {
+Table originalTable = testTables[0];
+
+EnvironmentContext context = new EnvironmentContext();
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY,
"transient_lastDdlTime");
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE,
"alma");
+
+client.alter_table(originalTable.getCatName(), originalTable.getDbName(),
originalTable.getTableName(),
+originalTable, context);
+ }
+
+ /**
+ * This tests ensures that concurrent Iceberg commits will fail. Acceptable
as a first sanity check.
+ *
+ * I have not found a good way to check that HMS side database commits are
parallel in the
+ * {@link
org.apache.hadoop.hive.metastore.HiveAlterHandler#alterTable(RawStore,
Warehouse, String, String, String, Table, EnvironmentContext, IHMSHandler,
String)}
+ * call, but this test could be used to manually ensure that using
breakpoints.
+ */
+ @Test
+ public void testAlterTableExpectedPropertyConcurrent() throws Exception {
+Table originalTable = testTables[0];
+
+originalTable.getParameters().put("snapshot", "1");
+client.alter_table(originalTable.getCatName(), originalTable.getDbName(),
originalTable.getTableName(),
+originalTable, null);
+
+EnvironmentContext context = new EnvironmentContext();
Review Comment:
Added a short loop (3), since in Derby the concurrent change causes a
deadlock and the deadlock timeout causes the test to run slowly.
Issue Time Tracking
---
Worklog Id: (was: 837016)
Time Spent: 1h 50m (was: 1h 40m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837001=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837001
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 04/Jan/23 18:45
Start Date: 04/Jan/23 18:45
Worklog Time Spent: 10m
Work Description: prasanthj commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1061784678
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java:
##
@@ -1183,6 +1193,90 @@ public void testAlterTableAlreadyExists() throws
Exception {
}
}
+ @Test
+ public void testAlterTableExpectedPropertyMatch() throws Exception {
+Table originalTable = testTables[0];
+
+EnvironmentContext context = new EnvironmentContext();
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY,
"transient_lastDdlTime");
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE,
+originalTable.getParameters().get("transient_lastDdlTime"));
+
+client.alter_table(originalTable.getCatName(), originalTable.getDbName(),
originalTable.getTableName(),
+originalTable, context);
+ }
+
+ @Test(expected = MetaException.class)
+ public void testAlterTableExpectedPropertyDifferent() throws Exception {
+Table originalTable = testTables[0];
+
+EnvironmentContext context = new EnvironmentContext();
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY,
"transient_lastDdlTime");
+context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE,
"alma");
+
+client.alter_table(originalTable.getCatName(), originalTable.getDbName(),
originalTable.getTableName(),
+originalTable, context);
+ }
+
+ /**
+ * This tests ensures that concurrent Iceberg commits will fail. Acceptable
as a first sanity check.
+ *
+ * I have not found a good way to check that HMS side database commits are
parallel in the
+ * {@link
org.apache.hadoop.hive.metastore.HiveAlterHandler#alterTable(RawStore,
Warehouse, String, String, String, Table, EnvironmentContext, IHMSHandler,
String)}
+ * call, but this test could be used to manually ensure that using
breakpoints.
+ */
+ @Test
+ public void testAlterTableExpectedPropertyConcurrent() throws Exception {
+Table originalTable = testTables[0];
+
+originalTable.getParameters().put("snapshot", "1");
+client.alter_table(originalTable.getCatName(), originalTable.getDbName(),
originalTable.getTableName(),
+originalTable, null);
+
+EnvironmentContext context = new EnvironmentContext();
Review Comment:
nit: small change to concurrency test. if you can run a loop for few
iterations with snapshot=i and expected value=i-1 that will be useful.. just
some repetition to avoid flakiness..
Issue Time Tracking
---
Worklog Id: (was: 837001)
Time Spent: 1h 40m (was: 1.5h)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=836620=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-836620 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 03/Jan/23 14:06 Start Date: 03/Jan/23 14:06 Worklog Time Spent: 10m Work Description: sonarcloud[bot] commented on PR #3888: URL: https://github.com/apache/hive/pull/3888#issuecomment-1369807428 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=3888) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=3888=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=3888=duplicated_lines_density=list) No Duplication information Issue Time Tracking --- Worklog Id: (was: 836620) Time Spent: 1.5h (was: 1h 20m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Priority: Major > Labels: pull-request-available > Time Spent: 1.5h > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=836617=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-836617
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 03/Jan/23 13:47
Start Date: 03/Jan/23 13:47
Worklog Time Spent: 10m
Work Description: pvary commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1060600376
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
It turns out, that you have been right. We use `READ_COMMITTED` isolation
level for our commits.
Because of this, we could commit concurrently, and lose data.
I had to set the isolation level for these transactions to `REPEATABLE_READ`
explicitly.
@szehon-ho and @prasanthj could you please check it if you are back from
holidays?
Thanks,
Peter
Issue Time Tracking
---
Worklog Id: (was: 836617)
Time Spent: 1h 20m (was: 1h 10m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=835448=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-835448
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 23/Dec/22 06:11
Start Date: 23/Dec/22 06:11
Worklog Time Spent: 10m
Work Description: pvary commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1056066482
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
Thankfully this is not true.
HMS does these changes using the underlying DB’s transaction.
See:
-
https://github.com/apache/hive/blob/84e65538e1009e163d8e81f8645aef9344e24499/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java#L180
-
https://github.com/apache/hive/blob/84e65538e1009e163d8e81f8645aef9344e24499/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L558
Issue Time Tracking
---
Worklog Id: (was: 835448)
Time Spent: 1h 10m (was: 1h)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=835447=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-835447
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 23/Dec/22 05:59
Start Date: 23/Dec/22 05:59
Worklog Time Spent: 10m
Work Description: pvary commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1056061034
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##
@@ -6038,17 +6039,26 @@ public void alter_table_with_environment_context(final
String dbname,
throws InvalidOperationException, MetaException {
String[] parsedDbName = parseDbName(dbname, conf);
alter_table_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME],
-name, newTable, envContext, null, null, null);
+name, newTable, envContext, null, null, null, null, null);
}
private void alter_table_core(String catName, String dbname, String name,
Table newTable,
-EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities, String processorId)
+EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities,
+String processorId, String
expectedPropertyKey, String expectedPropertyValue)
Review Comment:
Iceberg does not explicitly need this, and it would somewhat complicate the
code. So I voted down this. Still I do not feel too strongly either way, so if
you think it could be useful, I am open to add multiple checks based on a map
(do we have map in thrift?)
Issue Time Tracking
---
Worklog Id: (was: 835447)
Time Spent: 1h (was: 50m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=835410=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-835410
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 22/Dec/22 23:00
Start Date: 22/Dec/22 23:00
Worklog Time Spent: 10m
Work Description: szehon-ho commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1055915650
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
For my understanding, this only works if a user has table lock while calling
alter right?
If user does not have lock, Hive has no internal lock to prevent two users
from both getting their expected value and proceeding right?
Issue Time Tracking
---
Worklog Id: (was: 835410)
Time Spent: 50m (was: 40m)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[
https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=835409=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-835409
]
ASF GitHub Bot logged work on HIVE-26882:
-
Author: ASF GitHub Bot
Created on: 22/Dec/22 22:57
Start Date: 22/Dec/22 22:57
Worklog Time Spent: 10m
Work Description: szehon-ho commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1055913042
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##
@@ -6038,17 +6039,26 @@ public void alter_table_with_environment_context(final
String dbname,
throws InvalidOperationException, MetaException {
String[] parsedDbName = parseDbName(dbname, conf);
alter_table_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME],
-name, newTable, envContext, null, null, null);
+name, newTable, envContext, null, null, null, null, null);
}
private void alter_table_core(String catName, String dbname, String name,
Table newTable,
-EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities, String processorId)
+EnvironmentContext envContext, String
validWriteIdList, List processorCapabilities,
+String processorId, String
expectedPropertyKey, String expectedPropertyValue)
Review Comment:
Did you consider allowing a list of table properties here?
##
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
TableName.getQualified(catName, dbname, name) + " doesn't exist");
}
+ String expectedKey = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY)
: null;
+ String expectedValue = environmentContext != null &&
environmentContext.getProperties() != null ?
+
environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE)
: null;
+ if (expectedKey != null && expectedValue != null
+ && !expectedValue.equals(oldt.getParameters().get(expectedKey)))
{
+throw new MetaException("The table has been modified. The parameter
value for key '" + expectedKey + "' is '"
Review Comment:
For my understanding, this only works if a user has table lock while calling
alter right?
If user does not have lock, Hive has no internal lock to prevent two users
from both getting their expected value and proceeding right? If so, maybe
worth to add a javadoc on this method.
Issue Time Tracking
---
Worklog Id: (was: 835409)
Time Spent: 40m (was: 0.5h)
> Allow transactional check of Table parameter before altering the Table
> --
>
> Key: HIVE-26882
> URL: https://issues.apache.org/jira/browse/HIVE-26882
> Project: Hive
> Issue Type: Improvement
> Components: Standalone Metastore
>Reporter: Peter Vary
>Priority: Major
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
> We should add the possibility to transactionally check if a Table parameter
> is changed before altering the table in the HMS.
> This would provide an alternative, less error-prone and faster way to commit
> an Iceberg table, as the Iceberg table currently needs to:
> - Create an exclusive lock
> - Get the table metadata to check if the current snapshot is not changed
> - Update the table metadata
> - Release the lock
> After the change these 4 HMS calls could be substituted with a single alter
> table call.
> Also we could avoid cases where the locks are left hanging by failed processes
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=835150=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-835150 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 21/Dec/22 21:25 Start Date: 21/Dec/22 21:25 Worklog Time Spent: 10m Work Description: sonarcloud[bot] commented on PR #3888: URL: https://github.com/apache/hive/pull/3888#issuecomment-1362103589 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=3888) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=3888=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=3888=duplicated_lines_density=list) No Duplication information Issue Time Tracking --- Worklog Id: (was: 835150) Time Spent: 0.5h (was: 20m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Priority: Major > Labels: pull-request-available > Time Spent: 0.5h > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=835095=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-835095 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 21/Dec/22 16:06 Start Date: 21/Dec/22 16:06 Worklog Time Spent: 10m Work Description: sonarcloud[bot] commented on PR #3888: URL: https://github.com/apache/hive/pull/3888#issuecomment-1361580087 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=3888) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=3888=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=3888=duplicated_lines_density=list) No Duplication information Issue Time Tracking --- Worklog Id: (was: 835095) Time Spent: 20m (was: 10m) > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Priority: Major > Labels: pull-request-available > Time Spent: 20m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (HIVE-26882) Allow transactional check of Table parameter before altering the Table
[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=835019=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-835019 ] ASF GitHub Bot logged work on HIVE-26882: - Author: ASF GitHub Bot Created on: 21/Dec/22 10:19 Start Date: 21/Dec/22 10:19 Worklog Time Spent: 10m Work Description: sonarcloud[bot] commented on PR #3888: URL: https://github.com/apache/hive/pull/3888#issuecomment-1361125200 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=3888) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=3888=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive=3888=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=3888=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=3888=duplicated_lines_density=list) No Duplication information Issue Time Tracking --- Worklog Id: (was: 835019) Remaining Estimate: 0h Time Spent: 10m > Allow transactional check of Table parameter before altering the Table > -- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore >Reporter: Peter Vary >Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)
