You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "dengzhhu653 (via GitHub)" <gi...@apache.org> on 2025/07/04 10:36:48 UTC
[PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
dengzhhu653 opened a new pull request, #5929:
URL: https://github.com/apache/hive/pull/5929
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
No
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
Tested using the Repro.java against MySQL and Postgres, I didn't see the issue any more.
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3448462078
> -1
>
> > As far as I know in MySQL this will block others for the same row(the same for update), if the 0 row returns, means we have acquired the X tblId row lock, why do we need to retry again?
>
> Using retries instead of blocking aligns better with optimistic concurrency control, allowing transactions to proceed without waiting and only retry if a conflict actually occurs.
>
> The same approach is applied in several areas, including Iceberg’s conflict resolution ([HIVE-26882](https://issues.apache.org/jira/browse/HIVE-26882)) and compaction.
I don't think retry is the good way, it introduces another issue, the `COLUMN_STATS` in `TABLE_PARAMS` would be got overwritten, and lead to some column markers missing, so it might make the stats import useless.
For the updating the stats, we have already has the in-process lock, it doesn't solve the parallel imports across the HMS instances, somehow we need the distributed lock to achieve it, the DB lock is one way.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3455035790
I guess I'm a bit get your optimistic concurrency control, let me try
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 closed pull request #5929: HIVE-28578: Concurrency issue in updateTableColumnStatistics
URL: https://github.com/apache/hive/pull/5929
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3374726921
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2185168242
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
Why do we need a lock on "TBLS" when updating stats? Why not lock on `TAB_COL_STATS`?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2417829421
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
directSql.lockDbTable("TBLS") => MetaStoreDirectSql#lockDbTable() resolves to lock TBLS in exclusive mode, which prevents other users from reading or writing to the TBLS table. We'll have to pass a boolean argument to MetaStoreDirectSql#lockDbTable() to decide whether to lock the table exclusively or not.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2417973396
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9621,6 +9624,9 @@ public Map<String, String> updatePartitionColumnStatistics(Table table, MTable m
if (partition == null) {
throw new NoSuchObjectException("Partition for which stats is gathered doesn't exist.");
}
+ lockForUpdate("PARTITIONS", "PART_ID", "\"TBL_ID\" = " + mTable.getId()
Review Comment:
I thin, Only "NOTIFICATION_SEQUENCE" table should be exclusively locked and "TBLS", "PARTITIONS" tables can have Share/Row exclusive
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2185168242
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
Why do we need a lock on "TBLS" when updating stats?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2199316556
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
As far as I know it won't prevent others from reading the whole `TBLS`, the cases affected in HMS are dropping/altering this `TBL_ID` table, and dropping the database where this table sits.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2199316556
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
As far as I know it won't prevent others from reading the whole `TBLS`, the cases affected in HMS are dropping/altering this `TBL_ID` table.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3450844301
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3164076873
please give me some time to review and reply
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2430921626
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9621,6 +9624,9 @@ public Map<String, String> updatePartitionColumnStatistics(Table table, MTable m
if (partition == null) {
throw new NoSuchObjectException("Partition for which stats is gathered doesn't exist.");
}
+ lockForUpdate("PARTITIONS", "PART_ID", "\"TBL_ID\" = " + mTable.getId()
Review Comment:
this is for derby only `sqlGenerator.getDbProduct().isDERBY()`
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3451719433
> Currently the HMS is lack of versioning, the RDMS nowadays provides MVCC for writes not block any reads, so the write is the main concern here. The lock provides here is the row-level lock, it only blocks the write operation against the same table, such as the alter/drop the same table/partition, drop the db, or update the stats of this table/partition.
Every iceberg table commits involves alter table operation and it's non blocking ATM.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2418276655
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9621,6 +9624,9 @@ public Map<String, String> updatePartitionColumnStatistics(Table table, MTable m
if (partition == null) {
throw new NoSuchObjectException("Partition for which stats is gathered doesn't exist.");
}
+ lockForUpdate("PARTITIONS", "PART_ID", "\"TBL_ID\" = " + mTable.getId()
Review Comment:
This places an exclusive lock on the selected row: `select "PART_ID" from "PARTITIONS" where "TBL_ID" = " + mTable.getId()`
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9621,6 +9624,9 @@ public Map<String, String> updatePartitionColumnStatistics(Table table, MTable m
if (partition == null) {
throw new NoSuchObjectException("Partition for which stats is gathered doesn't exist.");
}
+ lockForUpdate("PARTITIONS", "PART_ID", "\"TBL_ID\" = " + mTable.getId()
Review Comment:
This places an exclusive lock on the selected row: `select "PART_ID" from "PARTITIONS" where "TBL_ID" = " + mTable.getId() for update`
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3379043315
@nrg4878 @saihemanth-cloudera @zhangbutao @wecharyu could you check this PR as well please?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2418272966
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
no, this `lockForUpdate`, effectively `select "TBL_ID" from "TBLS" where \"TBL_ID\" = " + mTable.getId() for update`, only places an exclusive lock on the matching row: `\"TBL_ID\" = " + mTable.getId()`, it won't prevent any read or the update on other rows.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3451199713
> I don't think retry is the good way, it introduces another issue, the `COLUMN_STATS` in `TABLE_PARAMS` would be got overwritten, and lead to some column markers missing, so it might make the stats import useless. For updating the stats, we have already had the in-process lock, it doesn't solve the parallel imports across the HMS instances, somehow we need the distributed lock to achieve it, the DB lock is one way.
Locking TBLS and PARTITIONS isn’t necessarily a better solution. In large-scale, highly concurrent systems, locking often becomes a scalability bottleneck. Instead, most systems adopt optimistic locking strategies. Each operation assumes no conflict and only validates at commit, reducing contention and avoiding blocking.
The idea is to balance consistency with throughput. Extending this principle to stats import (which btw is not a core functionality) we could consider a mechanism based on versioning rather than traditional locking. Such an approach generally scales far better in multi-instance HMS environments than depending solely on table or partition locks.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3037310258
There was another PR for the same: https://github.com/apache/hive/pull/5567
cc @InvisibleProgrammer
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2199316556
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
As far as I know it won't prevent others from reading the whole `TBLS`, the cases affected in HMS are dropping/altering this `TBL_ID` table, or dropping the database where this table sits.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3060469072
> > UPDATE TBS
> > SET last_updated = NOW()
> > WHERE TBL_ID = AND last_updated = ''; -- The exact timestamp you originally read
>
> As far as I know in MySQL this will block others for the same row(the same for update), if the 0 row returns, means we have acquired the X `tblId` row lock, why do we need to retry again?
>
> But this inspires me we can alter the table during the updating the column statistics, so we don't ask for the row lock explicitly, I will try and test this approach. Thank you!
After some reach, I found it's hard to use DataNucleus without the pure "UPDATE" query,
```java
mTable.setLastAccessTime((int) (System.currentTimeMillis()/1000));
pm.flush(); // here might flush the old MTable into data store
pm.refresh(mTable);
```
In `pm.flush()` the old state of the table can get overwritten the one in data store, resulting to some columns missing in COLUMN_STATS_ACCURATE, for example:
`{"COLUMN_STATS":{"col_0":"true","col_1":"true","col_2":"true","col_3":"true","col_5":"true","col_6":"true","col_7":"true","col_8":"true"}}`
`col_4` and `col_9` are missing.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2199316556
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
As far as I konw it won't prevent others from reading the whole `TBLS`, the cases affected in HMS are dropping/altering this `TBL_ID` table.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2418272966
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
no, this `lockForUpdate`, effectively `select * from "TBLS" where \"TBL_ID\" = " + mTable.getId() for update`, only places an exclusive lock on the matching row: `\"TBL_ID\" = " + mTable.getId()`, it won't prevent any read or the update on other rows.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2196996935
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
In this issue we have concurrency updates on the table params, locking on the `TAB_COL_STATS` (`TBL_ID`, `COLUMN_NAME`) is of no use, we still have the same issue when updating multi columns stats in parallel.
And the `TAB_COL_STATS` (`TBL_ID`) might not exist, so every client can take over the lock and proceed in this case, resulting to the same issue.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2185168242
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
Why do we need a lock on "TBLS" when updating stats? Why not lock on `COLUMNS_V2.CD_ID`?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3036232635
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2185168242
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
Why do we need a lock on "TBLS" when updating stats? Why not lock on `TAB_COL_STATS`?
can we check the result of `oldt.setParameters(newParams)` and retry when false?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2185168242
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
Why do we need a lock on "TBLS" when updating stats? Why not lock on `TAB_COL_STATS`?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2198194241
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
In that scenario, retrying seems like a better approach than placing a global lock on the commonly accessed `TBLS`.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3454010825
> -- MVCC style (optimistic)
BEGIN;
SELECT balance FROM accounts WHERE id = 1; -- snapshot read
-- some processing
UPDATE accounts SET balance = balance - 100 WHERE id = 1;
COMMIT;
-- If another transaction modified same row → commit fails (conflict)
As I explained, this would cause lost-update,
After some research, I find it's hard to use DataNucleus without the pure "UPDATE" query,
mTable.setLastAccessTime((int) (System.currentTimeMillis()/1000));
pm.flush(); // here might flush the old MTable into data store
pm.refresh(mTable);
In pm.flush() the old state of the table can get overwritten the one in data store, resulting to some columns missing in COLUMN_STATS_ACCURATE, for example:
{"COLUMN_STATS":{"col_0":"true","col_1":"true","col_2":"true","col_3":"true","col_5":"true","col_6":"true","col_7":"true","col_8":"true"}}
col_4 and col_9 are missing.
> Every iceberg table commits involves alter table operation and it's non-blocking ATM.
The Iceberg commit relies on the DB transaction atomicity, it should involve the row lock behind the scenes, though the lock is quite small(TBL_ID, PARAM_KEY), if the table has multiple commits at the same time, only one is allowed to alter the `TABLE_PARAMS`.
> That’s the opposite of what CU experienced on a highly loaded MySQL cluster with S4U on the NEXT_TXN_ID table.
This is because every HMS request needs to lock only one `NEXT_TXN_ID`, compared the same level lock distributed among different tables
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3449423189
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2196996935
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
In this issue we have concurrency updates on the table params, locking on the `TAB_COL_STATS` (`TBL_ID`, `COLUMN_NAME`) is of no use, we still have the same issue when updating multi columns stats in parallel.
And the `TAB_COL_STATS` (`TBL_ID`, `COLUMN_NAME`) might not exist, so every client can take over the lock and proceed in this case, resulting to the same issue.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2185160643
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -11448,36 +11454,34 @@ public List<WriteEventInfo> getAllWriteEventInfo(long txnId, String dbName, Stri
return writeEventInfoList;
}
- private void prepareQuotes() throws SQLException {
- String s = dbType.getPrepareTxnStmt();
- if (s != null) {
- assert pm.currentTransaction().isActive();
- JDOConnection jdoConn = pm.getDataStoreConnection();
- try (Statement statement = ((Connection) jdoConn.getNativeConnection()).createStatement()) {
- statement.execute(s);
- } finally {
- jdoConn.close();
- }
- }
- }
-
- private void lockNotificationSequenceForUpdate() throws MetaException {
+ private void lockForUpdate(String tableName, String column, String rowFilter)
Review Comment:
Wouldn't it be better with `Optional`?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2462637995
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9605,23 +9608,28 @@ public Map<String, String> updatePartitionColumnStatistics(Table table, MTable m
List<ColumnStatisticsObj> statsObjs = colStats.getStatsObj();
ColumnStatisticsDesc statsDesc = colStats.getStatsDesc();
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
- Partition partition = convertToPart(catName, statsDesc.getDbName(), statsDesc.getTableName(), getMPartition(
- catName, statsDesc.getDbName(), statsDesc.getTableName(), partVals, mTable), TxnUtils.isAcidTable(table));
+ MPartition mPartition = getMPartition(
+ catName, statsDesc.getDbName(), statsDesc.getTableName(), partVals, mTable);
+ Partition partition = convertToPart(catName, statsDesc.getDbName(), statsDesc.getTableName(),
+ mPartition, TxnUtils.isAcidTable(table));
List<String> colNames = new ArrayList<>();
for(ColumnStatisticsObj statsObj : statsObjs) {
colNames.add(statsObj.getColName());
}
- Map<String, MPartitionColumnStatistics> oldStats = getPartitionColStats(table, statsDesc
- .getPartName(), colNames, colStats.getEngine());
-
- MPartition mPartition = getMPartition(
- catName, statsDesc.getDbName(), statsDesc.getTableName(), partVals, mTable);
- if (partition == null) {
+ List<Long> partitionIds = directSql.getPartitionFieldsViaSqlFilter(catName, statsDesc.getDbName(), statsDesc.getTableName(),
+ Arrays.asList("\"PART_ID\""), "\"PARTITIONS\".\"PART_NAME\" = ?",
+ Arrays.asList(Warehouse.makePartName(table.getPartitionKeys(), partVals)), Collections.emptyList(), -1);
+ if (partition == null || partitionIds.isEmpty()) {
throw new NoSuchObjectException("Partition for which stats is gathered doesn't exist.");
}
+ Map<String, MPartitionColumnStatistics> oldStats = getPartitionColStats(table, statsDesc
+ .getPartName(), colNames, colStats.getEngine());
+ lockForUpdate("PARTITIONS", "PART_ID", Optional.of("\"PART_ID\" = " + partitionIds.getFirst()));
Review Comment:
change it to look up the partition by primary key
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3454638028
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3449686492
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2430735907
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9621,6 +9624,9 @@ public Map<String, String> updatePartitionColumnStatistics(Table table, MTable m
if (partition == null) {
throw new NoSuchObjectException("Partition for which stats is gathered doesn't exist.");
}
+ lockForUpdate("PARTITIONS", "PART_ID", "\"TBL_ID\" = " + mTable.getId()
Review Comment:
lockForUpdate() is calling directSql.lockDbTable(tableName)#L11464; and MetaStoreDirectSql#lockDbTable(String tableName) has this code
`String lockCommand = "lock table \"" + tableName + "\" in exclusive mode";
try {
executeNoResult(lockCommand);
} catch (SQLException sqle) {
throw new MetaException("Error while locking table " + tableName + ": " + sqle.getMessage());
}`
So we are indeed locking the table in exclusive mode, right?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3043467323
> There was another PR for the same: #5567 cc @InvisibleProgrammer I lean toward using retries rather than introducing locking. Non-blocking, better throughput under normal conditions.
>
> ```
> SqlRetryHandler sqlRetryHandler = new SqlRetryHandler(conf, dbType);
> ...
> return sqlRetryHandler.executeWithRetry(
> new SqlRetryCallProperties()
> .withCallerId("updateTableColumnStatistics")
> .withRetryOnDuplicateKey(true),
> () -> updateTableColumnStatisticsInternal(
> colStats, validWriteIds, writeId, catName, dbName, tableName));
> ```
>
> And since HMS provides `RetryingMetaStoreClient`, this might be redundant.
>
> -1 on the locking approach
Why locking approach isn't good?
Retries might bring another problem: it might cause the param key `COLUMN_STATS_ACCURATE` inaccurate or missing the marker for some columns, for example:
step 1: client1 has col1 updated, and set `COLUMN_STATS_ACCURATE` to `col1: true`
step 2: client2 for col2 and client3 for col3 both throw the duplicate key exception, then retry...
step 3: client2 and client 3 both read `COLUMN_STATS_ACCURATE` as `col1: true`, then update the param key separately.
step 4: After step 3 either `col2 : true` or `col3: true` is missing in `COLUMN_STATS_ACCURATE`.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2199316556
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9517,6 +9517,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
// So let's not use them anywhere unless absolutely necessary.
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
+ lockForUpdate("TBLS", "TBL_ID", "\"TBL_ID\" = " + mTable.getId());
Review Comment:
As far as I konw it won't prevent others from reading the whole `TBLS`, the cases affected are dropping/altering this `TBL_ID` table.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3059992046
> UPDATE TBS
SET last_updated = NOW()
WHERE TBL_ID = <tblId> AND last_updated = '<timestamp>'; -- The exact timestamp you originally read
As far as I know in mysql this will block others for the same row(the same for update), if the 0 row returns, means we have acquired the IX `tblId` row lock, why do we need to retry again?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3448034454
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5929&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5929&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5929&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5929)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3451454074
> > I don't think retry is the good way, it introduces another issue, the `COLUMN_STATS` in `TABLE_PARAMS` would be got overwritten, and lead to some column markers missing, so it might make the stats import useless. For updating the stats, we have already had the in-process lock, it doesn't solve the parallel imports across the HMS instances, somehow we need the distributed lock to achieve it, the DB lock is one way.
>
> Locking TBLS and PARTITIONS isn’t necessarily a better solution. In large-scale, highly concurrent systems, locking often becomes a scalability bottleneck. Instead, most systems adopt optimistic locking strategies. Each operation assumes no conflict and only validates at commit, reducing contention and avoiding blocking.
>
> The idea is to balance consistency with throughput. Extending this principle to stats import (which btw is not a core functionality) we could consider a mechanism based on versioning rather than traditional locking. Such an approach generally scales far better in multi-instance HMS environments than depending solely on table or partition locks.
Currently the HMS is lack of versioning, the RDMS nowadays provides MVCC for writes not block any reads, so the writes is the main concern here.
The lock provides here is the row-level lock, it only blocks the write operation against the same table, such as the alter/drop the same table/partition, drop the db, or update the stats of this table/partition.
Even without the explicit lock, we still have some exclusive row lock under the scenes, the RDMS in background ensures strong consistency and reliability the HMS benefits from and relies on, and provides a milliseconds to seconds query execution time, that can also have a satisfied throughput.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3449393825
For optimistic concurrency control, IMO this is more suitable for read skew scenario, otherwise it's a waste of the resource to retry on conflict
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5929:
URL: https://github.com/apache/hive/pull/5929#discussion_r2417834771
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -11448,36 +11454,34 @@ public List<WriteEventInfo> getAllWriteEventInfo(long txnId, String dbName, Stri
return writeEventInfoList;
}
- private void prepareQuotes() throws SQLException {
- String s = dbType.getPrepareTxnStmt();
- if (s != null) {
- assert pm.currentTransaction().isActive();
- JDOConnection jdoConn = pm.getDataStoreConnection();
- try (Statement statement = ((Connection) jdoConn.getNativeConnection()).createStatement()) {
- statement.execute(s);
- } finally {
- jdoConn.close();
- }
- }
- }
-
- private void lockNotificationSequenceForUpdate() throws MetaException {
+ private void lockForUpdate(String tableName, String column, String rowFilter)
Review Comment:
+1
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #5929:
URL: https://github.com/apache/hive/pull/5929#issuecomment-3466551049
Filed another PR: https://github.com/apache/hive/pull/6159 to catch the nowait for update
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org