You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "BsoBird (via GitHub)" <gi...@apache.org> on 2025/07/06 10:06:11 UTC
[PR] HIVE-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
BsoBird opened a new pull request, #5934:
URL: https://github.com/apache/hive/pull/5934
<!--
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.
-->
Modified the modifier of the cache property in the SplitGroup class to make it non-static.
JIRA: https://issues.apache.org/jira/browse/HIVE-27944
### 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.
-->
Since the Properties object implement HashTable interface, all the methods of the HashTable interface are synchronised.
In a multi-threaded environment, a deadlock will occur when propA.equals(propB) and propB.equals(propA) occur at the same time.
The problem can be solved with minimal modification by changing SplitGroup.cache to a non-static property.
### Does this PR introduce _any_ user-facing change?
<!--
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?
<!--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189567183
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> please take a look at https://github.com/apache/hive/pull/5935/files, it has some extra checkstyle fixes. If that looks OK, please re-apply here. TBH, I have no idea why in a world someone came up with this `Map<Map<>,Map<>>` data structure for a normalized path map.
I will update it later.
I guess the previous author added PartitionDesc to the map-key to maintain the partition information after the user modifies the partition.
For example, after modifying the partition, the user might have changed the path information of the partition. In the original structure, we could capture this change.
However, using a map to store the data is quite strange. Maybe he just wanted to use a Tuple2<Path, PartitionDesc>?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188304635
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ
`Multimap<Map<Path, PartitionDesc>, Path> cache`
In what scenarios would we use PartitionDesc in cache?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191831532
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, thanks for the details, it helped a lot. i managed to create a test that initializes the cache from `SplitGroper`. Note, I struggled to create an iceberg table backed by HadoopCatalog
>
> ```
> set iceberg.catalog.ice01.type=hadoop;
> set iceberg.catalog.ice01.warehouse=/tmp;
>
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Cannot set a custom location for a path-based table. Expected /tmp/default/orders but got file:/tmp/ice01/orders
>
> ```
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Table location not set
>
> Wonder how were you able to create one?
@deniskuzZ
Example:
```
----spark3.4.1+iceberg 1.4.2
CREATE TABLE IF NOT EXISTS datacenter.test.test_data_02 (
id string,name string
)
PARTITIONED BY (name,bucket(id,16))
TBLPROPERTIES ('read.orc.vectorization.enabled'='true','write.format.default'='orc','write.orc.bloom.filter.columns'='id','write.orc.compression-codec'='zstd','write.metadata.previous-versions-max'='3','write.metadata.delete-after-commit.enabled'='true')
STORED AS iceberg;
insert into datacenter.test.test_data_02(id,name) values('1','a'),('2','b');
--hive
CREATE EXTERNAL TABLE iceberg_dwd.test_data_02
STORED BY 'org.apache.iceberg.mr.hive.HiveIcebergStorageHandler'
LOCATION 'hdfs://xxxxxx/iceberg-catalog/warehouse/test/test_data_02'
TBLPROPERTIES ('iceberg.catalog'='location_based_table','engine.hive.enabled'='true');
```
But in reality, using Iceberg tables from other catalogs in HIVE is not always smooth. For example, if the table's location is set only using Iceberg properties without setting the HIVE table's LOCATION property, there's a high chance the table won't function properly.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191913274
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ
Sir, I have synchronized your changes, but I did not synchronize the modifications to HadoopCatalog. It may need to be split into another 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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2190730752
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
!!! Currently, this cache isn't useful for Iceberg tables because SplitGrouper doesn't receive any partition information. The dir path always points to the table directory, and the same applies to the keys in pathToPartitionInfo
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2190735698
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, please ping me once you sync the PR with https://github.com/apache/hive/pull/5935 and there is a green build.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188401163
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ
Also, I don't quite understand. The SplitGrouper object is released immediately after use. Is it necessary for us to use GuavaCache for caching?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ merged PR #5934:
URL: https://github.com/apache/hive/pull/5934
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3052961000
again some cluster 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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188343170
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
updated
````
From 1fa2dac1659cec3fdedb15a60fe269335a00c67a Mon Sep 17 00:00:00 2001
From: Denys Kuzmenko <de...@gmail.com>
Date: Sun, 6 Jul 2025 15:40:18 +0300
Subject: [PATCH 1/1] test
---
.../hadoop/hive/ql/exec/tez/SplitGrouper.java | 24 ++++++++----
.../hive/ql/io/HiveFileFormatUtils.java | 39 +++++++++----------
2 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
index 2ce7ed88c8..902fd232ed 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
@@ -29,11 +29,12 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
import org.apache.hadoop.hive.ql.io.AcidUtils;
import org.apache.hadoop.hive.ql.io.BucketizedHiveInputFormat;
-import org.apache.hadoop.mapred.InputFormat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hadoop.conf.Configuration;
@@ -69,10 +70,10 @@ public class SplitGrouper {
private static final Logger LOG = LoggerFactory.getLogger(SplitGrouper.class);
- // TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
- // can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private static final Cache<Path, Path> cache = Caffeine.newBuilder()
+ .maximumSize(1000000)
+ .expireAfterWrite(1, TimeUnit.DAYS)
+ .build();
private final TezMapredSplitsGrouper tezGrouper = new TezMapredSplitsGrouper();
@@ -412,8 +413,17 @@ private static MapWork populateMapWork(JobConf jobConf, String inputName) {
return work;
}
+ private static void checkAndClearCacheIfNeeded() {
+ if (cache.size() > MAX_CACHE_SIZE) {
+ cache.clear();
+ }
+ }
+
private boolean schemaEvolved(InputSplit s, InputSplit prevSplit, boolean groupAcrossFiles,
- MapWork work) throws IOException {
+ MapWork work) throws IOException {
+ // Check cache size periodically
+ checkAndClearCacheIfNeeded();
+
boolean retval = false;
Path path = ((FileSplit) s).getPath();
PartitionDesc pd = HiveFileFormatUtils.getFromPathRecursively(
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
index 20fde98c26..644ce9a3fd 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
@@ -360,17 +360,17 @@ private static RecordUpdater getRecordUpdater(JobConf jc,
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap) throws IOException {
+ Map<Path, Path> cacheMap) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema) throws IOException {
+ Map<Path, Path> cacheMap, boolean ignoreSchema) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, ignoreSchema, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema, boolean ifPresent)
+ Map<Path, Path> cacheMap, boolean ignoreSchema, boolean ifPresent)
throws IOException {
T part = getFromPath(pathToPartitionInfo, dir);
@@ -379,18 +379,7 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
|| (dir.toUri().getScheme() == null || dir.toUri().getScheme().trim().equals(""))
|| FileUtils.pathsContainNoScheme(pathToPartitionInfo.keySet()))) {
- Map<Path, T> newPathToPartitionInfo = null;
- if (cacheMap != null) {
- newPathToPartitionInfo = cacheMap.get(pathToPartitionInfo);
- }
-
- if (newPathToPartitionInfo == null) { // still null
- newPathToPartitionInfo = populateNewT(pathToPartitionInfo);
-
- if (cacheMap != null) {
- cacheMap.put(pathToPartitionInfo, newPathToPartitionInfo);
- }
- }
+ Map<Path, T> newPathToPartitionInfo = populateNewT(pathToPartitionInfo, cacheMap);
part = getFromPath(newPathToPartitionInfo, dir);
}
if (part != null || ifPresent) {
@@ -401,13 +390,21 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
}
}
- private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo) {
- Map<Path, T> newPathToPartitionInfo = new HashMap<>();
- for (Map.Entry<Path, T> entry: pathToPartitionInfo.entrySet()) {
- T partDesc = entry.getValue();
- Path pathOnly = Path.getPathWithoutSchemeAndAuthority(entry.getKey());
+ private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo,
+ Map<Path, Path> cacheMap) {
+ Map<Path, T> newPathToPartitionInfo = new HashMap<>(pathToPartitionInfo.size());
+
+ pathToPartitionInfo.forEach((originalPath, partDesc) -> {
+ Path pathOnly = cacheMap != null ?
+ cacheMap.get(originalPath) : null;
+ if (pathOnly == null) {
+ pathOnly = Path.getPathWithoutSchemeAndAuthority(originalPath);
+ if (cacheMap != null) {
+ cacheMap.put(originalPath, pathOnly);
+ }
+ }
newPathToPartitionInfo.put(pathOnly, partDesc);
- }
+ });
return newPathToPartitionInfo;
}
--
2.48.0
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191868462
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> !!! Currently, this cache isn't useful for Iceberg tables because SplitGrouper doesn't receive any partition information. The dir path always points to the table directory, and the same applies to the keys in pathToPartitionInfo
@deniskuzZ
You mean, even if we keep PartitionDesc, we shouldn't encounter a deadlock?
That's very strange. We did experience a deadlock when reading Iceberg tables before. We never encountered this issue when reading regular ORC tables.
However, at that time we were using HIVE 3.1.3 to read ICEBERG tables. Has this issue disappeared in HIVE4?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2192441217
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, perhaps you'd be interested in sharing your project on the Hive dev mailing list to attract contributors who might be interested in collaborating.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3057873497
rebased
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2192478773
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> You mean, even if we keep PartitionDesc, we shouldn't encounter a deadlock?
we might, that what you reproduced. I was just saying that PartitionDesc doesn't have much value for iceberg, at least right now.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2192478773
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> You mean, even if we keep PartitionDesc, we shouldn't encounter a deadlock?
we might, that what you reproduced. I was just saying that PartitionDesc doesn't have much value for iceberg.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189531594
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> how did you reproduce this in first place? Could you please share the query and tables definitions.
> I'm not trying to test for concurrency issues, I just want to trigger SplitGrouper in a way that uses the cache.
@deniskuzZ
Currently, my memory is a bit hazy. The necessary conditions to trigger this issue are as follows:
1.Use LLAP or a persistent Tez DAG(reuse on yarn) to read an Iceberg partition table.
2.The Iceberg table is just a regular Hadoop-catalog partitioned table.(But we may need to test the behavior when we set bucketing_version=1 or bucketing_version=2 for the Iceberg location based table..)
However, this issue is not consistently reproducible. We had been using IcebergStorageHandler for a long time without encountering this problem before.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193107853
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
- **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
- **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189008451
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, how did you reproduce this in first place? Could you please share the query and tables definitions.
I'm not trying to test for concurrency issues, I just want to trigger SplitGrouper in a way that uses the cache.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188386695
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ
Sir,How about WeakHashMap?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188296761
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
As I understand it, the goal here is simply to save on calls to `Path.getPathWithoutSchemeAndAuthority`
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3050961859
Faild tests unrelated this PR:
`Unable to create live FilePath for hive-precommit-pr-5934-5-mhfgv-v07p8-ftfcz; hive-precommit-pr-5934-5-mhfgv-v07p8-ftfcz was marked offline: Pod failed (Reason: Evicted, Message: Pod ephemeral local storage usage exceeds the total limit of containers 20Gi. )
`
BTW, have we reached an agreement on this PR?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188221837
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, can we refactor with MultiMap?
````
Subject: [PATCH] refactor
---
Index: ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java (revision ade51d810b383dd68874ecaa275c3a6cc9157884)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java (date 1751805392461)
@@ -23,6 +23,7 @@
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
@@ -32,6 +33,7 @@
import java.util.Properties;
import java.util.Set;
+import org.apache.commons.collections.MapUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
@@ -72,6 +74,7 @@
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Multimap;
/**
* An util class for various Hive file format tasks.
@@ -360,17 +363,17 @@
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap) throws IOException {
+ Multimap<Map<Path, T>, Path> cacheMap) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema) throws IOException {
+ Multimap<Map<Path, T>, Path> cacheMap, boolean ignoreSchema) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, ignoreSchema, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema, boolean ifPresent)
+ Multimap<Map<Path, T>, Path> cacheMap, boolean ignoreSchema, boolean ifPresent)
throws IOException {
T part = getFromPath(pathToPartitionInfo, dir);
@@ -381,14 +384,19 @@
Map<Path, T> newPathToPartitionInfo = null;
if (cacheMap != null) {
- newPathToPartitionInfo = cacheMap.get(pathToPartitionInfo);
+ Collection<Path> keys = cacheMap.get(pathToPartitionInfo);
+ if (keys != null) {
+ newPathToPartitionInfo =
+ populateNewT(keys, pathToPartitionInfo.values());
+ }
}
- if (newPathToPartitionInfo == null) { // still null
- newPathToPartitionInfo = populateNewT(pathToPartitionInfo);
+ if (MapUtils.isEmpty(newPathToPartitionInfo)) { // still null
+ newPathToPartitionInfo =
+ populateNewT(pathToPartitionInfo.keySet(), pathToPartitionInfo.values());
if (cacheMap != null) {
- cacheMap.put(pathToPartitionInfo, newPathToPartitionInfo);
+ cacheMap.putAll(pathToPartitionInfo, newPathToPartitionInfo.keySet());
}
}
part = getFromPath(newPathToPartitionInfo, dir);
@@ -401,12 +409,18 @@
}
}
- private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo) {
+ private static <T> Map<Path, T> populateNewT(Collection<Path> keys, Collection<T> values) {
+ if (keys.size() != values.size()) {
+ throw new IllegalArgumentException("Key and value collections must have the same size.");
+ }
Map<Path, T> newPathToPartitionInfo = new HashMap<>();
- for (Map.Entry<Path, T> entry: pathToPartitionInfo.entrySet()) {
- T partDesc = entry.getValue();
- Path pathOnly = Path.getPathWithoutSchemeAndAuthority(entry.getKey());
- newPathToPartitionInfo.put(pathOnly, partDesc);
+ Iterator<Path> keyIterator = keys.iterator();
+ Iterator<T> valueIterator = values.iterator();
+
+ while (keyIterator.hasNext() && valueIterator.hasNext()) {
+ Path key = Path.getPathWithoutSchemeAndAuthority(
+ keyIterator.next());
+ newPathToPartitionInfo.put(key, valueIterator.next());
}
return newPathToPartitionInfo;
}
Index: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java (revision ade51d810b383dd68874ecaa275c3a6cc9157884)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java (date 1751800901858)
@@ -71,8 +71,8 @@
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Multimap<Map<Path, PartitionDesc>, Path> cache =
+ ArrayListMultimap.create();
private final TezMapredSplitsGrouper tezGrouper = new TezMapredSplitsGrouper();
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193107853
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
HadoopTables: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3054386540
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_coverage&view=list)
 [3.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934)
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193987257
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
>
> * **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
> * **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
Sorry,Sir.
Because I previously reviewed the code and found that the submission and management logic of location_based_table is basically the same as that of hadoopCatalog. Therefore, I previously defaulted to treating location_based_table as HadoopCatalogTable. I confused the concepts, and this is my mistake.
Additionally, in the production environment, we currently only use HadoopTables (location_based_table) in HIVE.
(In Spark, I use hadoopCatalog to create a table. After Spark maintains this table, in HIVE I use HadoopTables to query the hadoopCatalog table maintained by Spark.)
Therefore, I only set the properties of IcebergHiveCatalog in HIVE, and did not set the related properties of other types of iceberg (including HadoopCatalog,Because HadoopTables can be used without any setup,so no need setup/use hadoopCatalog).
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189172246
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
please take a look at https://github.com/apache/hive/pull/5935/files, it has some extra checkstyle fixes. If that looks OK, please apply here.
TBH, I have no idea why in a world someone came up with this `Map<Map<>,Map<>>` data structure
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191930484
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ
BTW, since we're talking about HadoopCatalog, I'd like to recommend myself to show you a prototype project I previously used for research. It currently implements a reliable commit mechanism based on any file system. As HMS is trending towards being separated as a standalone metadata management tool, if it could have a built-in catalog that doesn't rely on DB or other services, but only a lightweight implementation dependent on the file system, I think many people would be willing to try it.
https://github.com/BsoBird/filesystem-catalog-original
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193987257
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
>
> * **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
> * **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
Sorry,Sir.
Because I previously reviewed the code and found that the submission and management logic of location_based_table is basically the same as that of hadoopCatalog. Therefore, I previously defaulted to treating location_based_table as HadoopCatalogTable. I confused the concepts, and this is my mistake.
Additionally, in the production environment, we currently only use HadoopTables (location_based_table) in HIVE.
(In Spark, I use hadoopCatalog to create a table. After Spark maintains this table, in HIVE I use HadoopTables to query the hadoopCatalog table maintained by Spark.)
Therefore, I only set the properties of IcebergHiveCatalog in HIVE, and did not set the related properties of other types of iceberg (including HadoopCatalog).
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191831532
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, thanks for the details, it helped a lot. i managed to create a test that initializes the cache from `SplitGroper`. Note, I struggled to create an iceberg table backed by HadoopCatalog
>
> ```
> set iceberg.catalog.ice01.type=hadoop;
> set iceberg.catalog.ice01.warehouse=/tmp;
>
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Cannot set a custom location for a path-based table. Expected /tmp/default/orders but got file:/tmp/ice01/orders
>
> ```
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Table location not set
>
> Wonder how were you able to create one?
@deniskuzZ
Example:
```
----spark3.4.1+iceberg 1.4.2
CREATE TABLE IF NOT EXISTS datacenter.test.test_data_02 (
id string,name string
)
PARTITIONED BY (name)
TBLPROPERTIES ('read.orc.vectorization.enabled'='true','write.format.default'='orc','write.orc.bloom.filter.columns'='id','write.orc.compression-codec'='zstd','write.metadata.previous-versions-max'='3','write.metadata.delete-after-commit.enabled'='true')
STORED AS iceberg;
insert into datacenter.test.test_data_02(id,name) values('1','a'),('2','b');
--hive
CREATE EXTERNAL TABLE iceberg_dwd.test_data_02
STORED BY 'org.apache.iceberg.mr.hive.HiveIcebergStorageHandler'
LOCATION 'hdfs://xxxxxx/iceberg-catalog/warehouse/test/test_data_02'
TBLPROPERTIES ('iceberg.catalog'='location_based_table','engine.hive.enabled'='true');
```
But in reality, using Iceberg tables from other catalogs in HIVE is not always smooth. For example, if the table's location is set only using Iceberg properties without setting the HIVE table's LOCATION property, there's a high chance the table won't function properly.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188516410
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
I've been trying to verify the cache is being used through SplitGrouper, but so far without success.
`dir.toUri().getScheme()` is not null or empty
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188343170
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
updated
````
From c62bbba350e9168d5bab4e727f1f5e18ff8fc26d Mon Sep 17 00:00:00 2001
From: Denys Kuzmenko <de...@gmail.com>
Date: Sun, 6 Jul 2025 15:40:18 +0300
Subject: [PATCH 1/1] test
---
.../hadoop/hive/ql/exec/tez/SplitGrouper.java | 5 +--
.../hive/ql/io/HiveFileFormatUtils.java | 39 +++++++++----------
2 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
index 2ce7ed88c8..f867c251a8 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
@@ -69,10 +69,7 @@ public class SplitGrouper {
private static final Logger LOG = LoggerFactory.getLogger(SplitGrouper.class);
- // TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
- // can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Path, Path> cache = new ConcurrentHashMap<>();
private final TezMapredSplitsGrouper tezGrouper = new TezMapredSplitsGrouper();
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
index 20fde98c26..644ce9a3fd 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
@@ -360,17 +360,17 @@ private static RecordUpdater getRecordUpdater(JobConf jc,
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap) throws IOException {
+ Map<Path, Path> cacheMap) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema) throws IOException {
+ Map<Path, Path> cacheMap, boolean ignoreSchema) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, ignoreSchema, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema, boolean ifPresent)
+ Map<Path, Path> cacheMap, boolean ignoreSchema, boolean ifPresent)
throws IOException {
T part = getFromPath(pathToPartitionInfo, dir);
@@ -379,18 +379,7 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
|| (dir.toUri().getScheme() == null || dir.toUri().getScheme().trim().equals(""))
|| FileUtils.pathsContainNoScheme(pathToPartitionInfo.keySet()))) {
- Map<Path, T> newPathToPartitionInfo = null;
- if (cacheMap != null) {
- newPathToPartitionInfo = cacheMap.get(pathToPartitionInfo);
- }
-
- if (newPathToPartitionInfo == null) { // still null
- newPathToPartitionInfo = populateNewT(pathToPartitionInfo);
-
- if (cacheMap != null) {
- cacheMap.put(pathToPartitionInfo, newPathToPartitionInfo);
- }
- }
+ Map<Path, T> newPathToPartitionInfo = populateNewT(pathToPartitionInfo, cacheMap);
part = getFromPath(newPathToPartitionInfo, dir);
}
if (part != null || ifPresent) {
@@ -401,13 +390,21 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
}
}
- private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo) {
- Map<Path, T> newPathToPartitionInfo = new HashMap<>();
- for (Map.Entry<Path, T> entry: pathToPartitionInfo.entrySet()) {
- T partDesc = entry.getValue();
- Path pathOnly = Path.getPathWithoutSchemeAndAuthority(entry.getKey());
+ private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo,
+ Map<Path, Path> cacheMap) {
+ Map<Path, T> newPathToPartitionInfo = new HashMap<>(pathToPartitionInfo.size());
+
+ pathToPartitionInfo.forEach((originalPath, partDesc) -> {
+ Path pathOnly = cacheMap != null ?
+ cacheMap.get(originalPath) : null;
+ if (pathOnly == null) {
+ pathOnly = Path.getPathWithoutSchemeAndAuthority(originalPath);
+ if (cacheMap != null) {
+ cacheMap.put(originalPath, pathOnly);
+ }
+ }
newPathToPartitionInfo.put(pathOnly, partDesc);
- }
+ });
return newPathToPartitionInfo;
}
--
2.48.0
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2192893820
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> could you please share configs you've used for 'iceberg.catalog'='location_based_table'? example:
>
> ```
> set iceberg.catalog.ice01.type=hadoop;
> set iceberg.catalog.ice01.warehouse=/tmp;
> ```
@deniskuzZ
no need config,just use this:
```
CREATE EXTERNAL TABLE iceberg_dwd.test_data_02
STORED BY 'org.apache.iceberg.mr.hive.HiveIcebergStorageHandler'
LOCATION 'hdfs://xxxxxx/iceberg-catalog/warehouse/test/test_data_02'
TBLPROPERTIES ('iceberg.catalog'='location_based_table','engine.hive.enabled'='true');
```
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193107853
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
- HadoopTables: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
- HadoopCatalog: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2190553680
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, thanks for the details, it helped a lot. i managed to create a test that initializes the cache from `SplitGroper`.
Note, I struggled to create an iceberg table backed by HadoopCatalog
````
set iceberg.catalog.ice01.type=hadoop;
set iceberg.catalog.ice01.warehouse=/tmp;
CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
PARTITIONED BY (p1 STRING, p2 STRING)
STORED BY ICEBERG STORED AS ORC
LOCATION '/tmp/ice01/orders'
TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
````
Caused by: java.lang.IllegalArgumentException: Cannot set a custom location for a path-based table. Expected /tmp/default/orders but got file:/tmp/ice01/orders
````
CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
PARTITIONED BY (p1 STRING, p2 STRING)
STORED BY ICEBERG STORED AS ORC
LOCATION '/tmp/ice01/orders'
TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
````
Caused by: java.lang.IllegalArgumentException: Table location not set
Wonder how did were you able to create one?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3057440307
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_coverage&view=list)
 [3.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934)
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3052120548
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_coverage&view=list)
 [3.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934)
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3053461769
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_coverage&view=list)
 [3.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934)
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188159918
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
please replace with Multimap
````
private final Multimap<Map<Path, PartitionDesc>, PartitionDesc> cache =
ArrayListMultimap.create();
````
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird please replace with Multimap
````
private final Multimap<Map<Path, PartitionDesc>, PartitionDesc> cache =
ArrayListMultimap.create();
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189531594
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> how did you reproduce this in first place? Could you please share the query and tables definitions.
> I'm not trying to test for concurrency issues, I just want to trigger SplitGrouper in a way that uses the cache.
@deniskuzZ
Currently, my memory is a bit hazy. The necessary conditions to trigger this issue are as follows:
1.Use LLAP or a persistent Tez DAG(reuse on yarn) to read an Iceberg partition table.
2.The Iceberg table is just a regular Hadoop-catalog partitioned table.(But we may need to test the behavior when we set bucketing_version=1 or bucketing_version=2 for the Iceberg external table..)
However, this issue is not consistently reproducible. We had been using IcebergStorageHandler for a long time without encountering this problem before.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188419111
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
i reverted to static as it was a global cache before
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188516410
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
I've been trying to verify the cache is being used through SplitGrouper, but so far without success.
`if (part == null)` returns 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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188221837
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, can we refactor with MultiMap?
````
Subject: [PATCH] refactor
---
Index: ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java (revision ade51d810b383dd68874ecaa275c3a6cc9157884)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java (date 1751805169036)
@@ -23,6 +23,7 @@
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
@@ -32,6 +33,8 @@
import java.util.Properties;
import java.util.Set;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.MapUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
@@ -72,6 +75,7 @@
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Multimap;
/**
* An util class for various Hive file format tasks.
@@ -360,17 +364,17 @@
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap) throws IOException {
+ Multimap<Map<Path, T>, Path> cacheMap) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema) throws IOException {
+ Multimap<Map<Path, T>, Path> cacheMap, boolean ignoreSchema) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, ignoreSchema, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema, boolean ifPresent)
+ Multimap<Map<Path, T>, Path> cacheMap, boolean ignoreSchema, boolean ifPresent)
throws IOException {
T part = getFromPath(pathToPartitionInfo, dir);
@@ -381,14 +385,19 @@
Map<Path, T> newPathToPartitionInfo = null;
if (cacheMap != null) {
- newPathToPartitionInfo = cacheMap.get(pathToPartitionInfo);
+ Collection<Path> keys = cacheMap.get(pathToPartitionInfo);
+ if (keys != null) {
+ newPathToPartitionInfo =
+ populateNewT(keys, pathToPartitionInfo.values());
+ }
}
- if (newPathToPartitionInfo == null) { // still null
- newPathToPartitionInfo = populateNewT(pathToPartitionInfo);
+ if (MapUtils.isEmpty(newPathToPartitionInfo)) { // still null
+ newPathToPartitionInfo =
+ populateNewT(pathToPartitionInfo.keySet(), pathToPartitionInfo.values());
if (cacheMap != null) {
- cacheMap.put(pathToPartitionInfo, newPathToPartitionInfo);
+ cacheMap.putAll(pathToPartitionInfo, newPathToPartitionInfo.keySet());
}
}
part = getFromPath(newPathToPartitionInfo, dir);
@@ -401,12 +410,18 @@
}
}
- private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo) {
+ private static <T> Map<Path, T> populateNewT(Collection<Path> keys, Collection<T> values) {
+ if (keys.size() != values.size()) {
+ throw new IllegalArgumentException("Key and value collections must have the same size.");
+ }
Map<Path, T> newPathToPartitionInfo = new HashMap<>();
- for (Map.Entry<Path, T> entry: pathToPartitionInfo.entrySet()) {
- T partDesc = entry.getValue();
- Path pathOnly = Path.getPathWithoutSchemeAndAuthority(entry.getKey());
- newPathToPartitionInfo.put(pathOnly, partDesc);
+ Iterator<Path> keyIterator = keys.iterator();
+ Iterator<T> valueIterator = values.iterator();
+
+ while (keyIterator.hasNext() && valueIterator.hasNext()) {
+ Path key = Path.getPathWithoutSchemeAndAuthority(
+ keyIterator.next());
+ newPathToPartitionInfo.put(key, valueIterator.next());
}
return newPathToPartitionInfo;
}
Index: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java (revision ade51d810b383dd68874ecaa275c3a6cc9157884)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java (date 1751800901858)
@@ -71,8 +71,8 @@
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Multimap<Map<Path, PartitionDesc>, Path> cache =
+ ArrayListMultimap.create();
private final TezMapredSplitsGrouper tezGrouper = new TezMapredSplitsGrouper();
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189172246
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
please take a look at https://github.com/apache/hive/pull/5935/files, it has some extra checkstyle fixes. If that looks OK, please apply here.
TBH, I have no idea why in a world someone came up with this `Map<Map<>,Map<>>` data structure for a normalized path map.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191831532
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, thanks for the details, it helped a lot. i managed to create a test that initializes the cache from `SplitGroper`. Note, I struggled to create an iceberg table backed by HadoopCatalog
>
> ```
> set iceberg.catalog.ice01.type=hadoop;
> set iceberg.catalog.ice01.warehouse=/tmp;
>
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Cannot set a custom location for a path-based table. Expected /tmp/default/orders but got file:/tmp/ice01/orders
>
> ```
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Table location not set
>
> Wonder how were you able to create one?
@deniskuzZ
Example:
```
----spark3.4.1+iceberg 1.4.2
CREATE TABLE IF NOT EXISTS datacenter.test.test_data_02 (
id string,name string
)
PARTITIONED BY (name)
TBLPROPERTIES ('read.orc.vectorization.enabled'='true','write.format.default'='orc','write.orc.bloom.filter.columns'='id','write.orc.compression-codec'='zstd','write.metadata.previous-versions-max'='3','write.metadata.delete-after-commit.enabled'='true')
STORED AS iceberg;
insert into datacenter.test.test_data_02(id,name) values('1','a'),('2','b');
--hive
CREATE EXTERNAL TABLE iceberg_dwd.test_data_02
STORED BY 'org.apache.iceberg.mr.hive.HiveIcebergStorageHandler'
LOCATION 'hdfs://xxxxxx/iceberg-catalog/warehouse/test/test_data_02'
TBLPROPERTIES ('iceberg.catalog'='location_based_table','engine.hive.enabled'='true');
```
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191868462
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> !!! Currently, this cache isn't useful for Iceberg tables because SplitGrouper doesn't receive any partition information. The dir path always points to the table directory, and the same applies to the keys in pathToPartitionInfo
@deniskuzZ
You mean, even if we keep PartitionDesc, we shouldn't encounter a deadlock?
That's very strange. We did experience a deadlock when reading Iceberg tables before. We never encountered this issue when reading regular ORC tables.
However, at that time we were using HIVE 3.1.3 to read ICEBERG tables. Has this issue disappeared in HIVE4?
However, no matter what, I also agree that SplitGrouper.cache should not use a strange data structure like Map<Map<>,Map<>>.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193987257
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
>
> * **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
> * **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
Sorry,Sir.
Because I previously reviewed the code and found that the submission and management logic of location_based_table is basically the same as that of hadoopCatalog. Therefore, I previously defaulted to treating location_based_table as HadoopCatalogTable. I confused the concepts, and this is my mistake.
Additionally, in the production environment, we currently only use HadoopTables (location_based_table) in HIVE.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3048903081
@BsoBird, you need to rebase with master, it's on jdk21 now
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3057555293
I just stopped the precommits since they failed due to resource limits. Make sure to rebase the PR before relaunching.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2194282325
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
HadoopCatalog is generally a better choice for production environments, as it offers namespace management, table discovery, and safe concurrent access.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3058680011
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_coverage&view=list)
 [3.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934)
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193987257
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
>
> * **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
> * **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
Sorry,Sir.
Because I previously reviewed the code and found that the submission and management logic of location_based_table is basically the same as that of hadoopCatalog. Therefore, I previously defaulted to treating location_based_table as HadoopCatalogTable. I confused the concepts, and this is my mistake.
Additionally, in the production environment, we currently only use HadoopTables (location_based_table) in HIVE.
(In Spark, I use hadoopCatalog to create a table. After Spark maintains this table, in HIVE I use HadoopTables to query the hadoopCatalog table maintained by Spark.)
Therefore, I only set the properties of IcebergHiveCatalog in HIVE, and did not set the related properties of other types of iceberg (including HadoopCatalog,Because HadoopTables can be used without any setup,so no need setup hadoopCatalog).
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3051066966
> Faild tests unrelated this PR: `Unable to create live FilePath for hive-precommit-pr-5934-5-mhfgv-v07p8-ftfcz; hive-precommit-pr-5934-5-mhfgv-v07p8-ftfcz was marked offline: Pod failed (Reason: Evicted, Message: Pod ephemeral local storage usage exceeds the total limit of containers 20Gi. ) `
>
> BTW, have we reached an agreement on this PR?
@zhangbutao
i think,yes.
Because in any case, this strange caching structure in SplitGrouper should no longer be used.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3050328149
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5934&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5934&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_coverage&view=list)
 [3.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5934&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5934)
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188419111
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
replaced with `IOPrepareCache.get().allocatePartitionDescMap()`
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188159918
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird please replace with Multimap
````
private final Multimap<Map<Path, PartitionDesc>, PartitionDesc> cache =
ArrayListMultimap.create();
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188221837
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, can we refactor with MultiMap?
````
Subject: [PATCH] refactor
---
Index: ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java (revision ade51d810b383dd68874ecaa275c3a6cc9157884)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java (date 1751803821678)
@@ -23,6 +23,7 @@
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
@@ -32,6 +33,7 @@
import java.util.Properties;
import java.util.Set;
+import org.apache.commons.collections.MapUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
@@ -72,6 +74,7 @@
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Multimap;
/**
* An util class for various Hive file format tasks.
@@ -360,17 +363,17 @@
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap) throws IOException {
+ Multimap<Map<Path, T>, Path> cacheMap) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema) throws IOException {
+ Multimap<Map<Path, T>, Path> cacheMap, boolean ignoreSchema) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, ignoreSchema, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema, boolean ifPresent)
+ Multimap<Map<Path, T>, Path> cacheMap, boolean ignoreSchema, boolean ifPresent)
throws IOException {
T part = getFromPath(pathToPartitionInfo, dir);
@@ -381,14 +384,16 @@
Map<Path, T> newPathToPartitionInfo = null;
if (cacheMap != null) {
- newPathToPartitionInfo = cacheMap.get(pathToPartitionInfo);
+ newPathToPartitionInfo =
+ populateNewT(cacheMap.get(pathToPartitionInfo), pathToPartitionInfo.values());
}
- if (newPathToPartitionInfo == null) { // still null
- newPathToPartitionInfo = populateNewT(pathToPartitionInfo);
+ if (MapUtils.isEmpty(newPathToPartitionInfo)) { // still null
+ newPathToPartitionInfo =
+ populateNewT(pathToPartitionInfo.keySet(), pathToPartitionInfo.values());
if (cacheMap != null) {
- cacheMap.put(pathToPartitionInfo, newPathToPartitionInfo);
+ cacheMap.putAll(pathToPartitionInfo, newPathToPartitionInfo.keySet());
}
}
part = getFromPath(newPathToPartitionInfo, dir);
@@ -401,12 +406,18 @@
}
}
- private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo) {
+ private static <T> Map<Path, T> populateNewT(Collection<Path> keys, Collection<T> values) {
+ if (keys.size() != values.size()) {
+ throw new IllegalArgumentException("Key and value collections must have the same size.");
+ }
Map<Path, T> newPathToPartitionInfo = new HashMap<>();
- for (Map.Entry<Path, T> entry: pathToPartitionInfo.entrySet()) {
- T partDesc = entry.getValue();
- Path pathOnly = Path.getPathWithoutSchemeAndAuthority(entry.getKey());
- newPathToPartitionInfo.put(pathOnly, partDesc);
+ Iterator<Path> keyIterator = keys.iterator();
+ Iterator<T> valueIterator = values.iterator();
+
+ while (keyIterator.hasNext() && valueIterator.hasNext()) {
+ Path key = Path.getPathWithoutSchemeAndAuthority(
+ keyIterator.next());
+ newPathToPartitionInfo.put(key, valueIterator.next());
}
return newPathToPartitionInfo;
}
Index: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java (revision ade51d810b383dd68874ecaa275c3a6cc9157884)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java (date 1751800901858)
@@ -71,8 +71,8 @@
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Multimap<Map<Path, PartitionDesc>, Path> cache =
+ ArrayListMultimap.create();
private final TezMapredSplitsGrouper tezGrouper = new TezMapredSplitsGrouper();
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188254512
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ It would be even better if you could explain why we need to use multiMap.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188419111
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
i reverted to static
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193987257
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
>
> * **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
> * **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
Sorry,Sir.
Because I previously reviewed the code and found that the submission and management logic of location_based_table is basically the same as that of hadoopCatalog. Therefore, I previously defaulted to treating location_based_table as HadoopCatalogTable. I confused the concepts, and this is my mistake.
Additionally, in the production environment, we currently only use HadoopTables (location_based_table) in HIVE.
(In Spark, I use hadoopCatalog to create a table. After Spark maintains this table, in HIVE I use HadoopTables to query the hadoopCatalog table maintained by Spark.)
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188253969
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ
At present, only one code is involved:
```
// IOPrepareCache.get().allocatePartitionDescMap()
public Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> allocatePartitionDescMap() {
if (partitionDescMap == null) {
partitionDescMap = new HashMap<>();
}
return partitionDescMap;
}
```
usage demo:
```
// extract all the inputFormatClass names for each chunk in the
// CombinedSplit.
PartitionDesc part = HiveFileFormatUtils.getFromPathRecursively(pathToPartitionInfo,
inputSplitShim.getPath(0), IOPrepareCache.get().getPartitionDescMap());
```
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188343170
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
updated
````
From 1fa2dac1659cec3fdedb15a60fe269335a00c67a Mon Sep 17 00:00:00 2001
From: Denys Kuzmenko <de...@gmail.com>
Date: Sun, 6 Jul 2025 15:40:18 +0300
Subject: [PATCH 1/1] test
---
.../hadoop/hive/ql/exec/tez/SplitGrouper.java | 24 ++++++++----
.../hive/ql/io/HiveFileFormatUtils.java | 39 +++++++++----------
2 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
index 2ce7ed88c8..902fd232ed 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
@@ -29,11 +29,12 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
import org.apache.hadoop.hive.ql.io.AcidUtils;
import org.apache.hadoop.hive.ql.io.BucketizedHiveInputFormat;
-import org.apache.hadoop.mapred.InputFormat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hadoop.conf.Configuration;
@@ -69,10 +70,10 @@ public class SplitGrouper {
private static final Logger LOG = LoggerFactory.getLogger(SplitGrouper.class);
- // TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
- // can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private static final Cache<Path, Path> cache = Caffeine.newBuilder()
+ .maximumSize(10000000)
+ .expireAfterWrite(1, TimeUnit.DAYS)
+ .build();
private final TezMapredSplitsGrouper tezGrouper = new TezMapredSplitsGrouper();
@@ -412,8 +413,17 @@ private static MapWork populateMapWork(JobConf jobConf, String inputName) {
return work;
}
+ private static void checkAndClearCacheIfNeeded() {
+ if (cache.size() > MAX_CACHE_SIZE) {
+ cache.clear();
+ }
+ }
+
private boolean schemaEvolved(InputSplit s, InputSplit prevSplit, boolean groupAcrossFiles,
- MapWork work) throws IOException {
+ MapWork work) throws IOException {
+ // Check cache size periodically
+ checkAndClearCacheIfNeeded();
+
boolean retval = false;
Path path = ((FileSplit) s).getPath();
PartitionDesc pd = HiveFileFormatUtils.getFromPathRecursively(
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
index 20fde98c26..644ce9a3fd 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
@@ -360,17 +360,17 @@ private static RecordUpdater getRecordUpdater(JobConf jc,
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap) throws IOException {
+ Map<Path, Path> cacheMap) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema) throws IOException {
+ Map<Path, Path> cacheMap, boolean ignoreSchema) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, ignoreSchema, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema, boolean ifPresent)
+ Map<Path, Path> cacheMap, boolean ignoreSchema, boolean ifPresent)
throws IOException {
T part = getFromPath(pathToPartitionInfo, dir);
@@ -379,18 +379,7 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
|| (dir.toUri().getScheme() == null || dir.toUri().getScheme().trim().equals(""))
|| FileUtils.pathsContainNoScheme(pathToPartitionInfo.keySet()))) {
- Map<Path, T> newPathToPartitionInfo = null;
- if (cacheMap != null) {
- newPathToPartitionInfo = cacheMap.get(pathToPartitionInfo);
- }
-
- if (newPathToPartitionInfo == null) { // still null
- newPathToPartitionInfo = populateNewT(pathToPartitionInfo);
-
- if (cacheMap != null) {
- cacheMap.put(pathToPartitionInfo, newPathToPartitionInfo);
- }
- }
+ Map<Path, T> newPathToPartitionInfo = populateNewT(pathToPartitionInfo, cacheMap);
part = getFromPath(newPathToPartitionInfo, dir);
}
if (part != null || ifPresent) {
@@ -401,13 +390,21 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
}
}
- private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo) {
- Map<Path, T> newPathToPartitionInfo = new HashMap<>();
- for (Map.Entry<Path, T> entry: pathToPartitionInfo.entrySet()) {
- T partDesc = entry.getValue();
- Path pathOnly = Path.getPathWithoutSchemeAndAuthority(entry.getKey());
+ private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo,
+ Map<Path, Path> cacheMap) {
+ Map<Path, T> newPathToPartitionInfo = new HashMap<>(pathToPartitionInfo.size());
+
+ pathToPartitionInfo.forEach((originalPath, partDesc) -> {
+ Path pathOnly = cacheMap != null ?
+ cacheMap.get(originalPath) : null;
+ if (pathOnly == null) {
+ pathOnly = Path.getPathWithoutSchemeAndAuthority(originalPath);
+ if (cacheMap != null) {
+ cacheMap.put(originalPath, pathOnly);
+ }
+ }
newPathToPartitionInfo.put(pathOnly, partDesc);
- }
+ });
return newPathToPartitionInfo;
}
--
2.48.0
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188426666
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@deniskuzZ
> i reverted to static as it was a global cache before
So, if I do this, is it possible that it will cause splitGrouper to produce incorrect calculation results? Because no one updates it.
```
alter table xxx drop partition([pt=11);
alter table xxx add partition([pt=11) location '/p1';
alter table xxx drop partition([pt=11);
alter table xxx add partition([pt=11) location '/p2';
alter table xxx drop partition([pt=11);
alter table xxx add partition([pt=11) location '/p1';
```
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2190566873
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> I guess the previous author added PartitionDesc to the map-key to maintain the partition information after the user modifies the partition. For example, after modifying the partition, the user might have changed the path information of the partition. In the original structure, we could capture this change. However, using a map to store the data is quite strange. Maybe he just wanted to use a Tuple2<Path, PartitionDesc>?
I don't follow. If the user changes the path - he won't be able to do cache lookup and need to create new path mapping. I still don't get why PartitionDesc is needed here at all
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189531594
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> how did you reproduce this in first place? Could you please share the query and tables definitions.
> I'm not trying to test for concurrency issues, I just want to trigger SplitGrouper in a way that uses the cache.
@deniskuzZ
Currently, my memory is a bit hazy. The necessary conditions to trigger this issue are as follows:
1.Use LLAP or a persistent Tez DAG(reuse on yarn) to read an Iceberg table.
2.The Iceberg table is just a regular Hadoop-catalog partitioned table.
However, this issue is not consistently reproducible. We had been using IcebergStorageHandler for a long time without encountering this problem before.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5934:
URL: https://github.com/apache/hive/pull/5934#issuecomment-3051502056
re-triggered the build
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189172246
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
please take a look at https://github.com/apache/hive/pull/5935/files, it has some extra checkstyle fixes. If that looks OK, please re-apply here.
TBH, I have no idea why in a world someone came up with this `Map<Map<>,Map<>>` data structure for a normalized path map.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189172246
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
please take a look at https://github.com/apache/hive/pull/5935/files, it has some extra checkstyle fixes. If it looks OK, please apply here.
TBH, I have no idea why in a world someone came up with this `Map<Map<>,Map<>>` data structure
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2192472212
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
could you please share configs you've used for 'iceberg.catalog'='location_based_table'?
example:
```
set iceberg.catalog.ice01.type=hadoop;
set iceberg.catalog.ice01.warehouse=/tmp;
```
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188290330
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
Maybe we don’t need it. Are you suggesting that Map<Path, Path> would be sufficient?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188339153
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
my guess was that we wanted to keep the cache based on PartitionDesc in case it changes
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2188383682
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, please take a look at https://github.com/apache/hive/pull/5935. If it's ok, please update the PR
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
updated
````
From 1fa2dac1659cec3fdedb15a60fe269335a00c67a Mon Sep 17 00:00:00 2001
From: Denys Kuzmenko <de...@gmail.com>
Date: Sun, 6 Jul 2025 15:40:18 +0300
Subject: [PATCH 1/1] test
---
.../hadoop/hive/ql/exec/tez/SplitGrouper.java | 24 ++++++++----
.../hive/ql/io/HiveFileFormatUtils.java | 39 +++++++++----------
2 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
index 2ce7ed88c8..902fd232ed 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java
@@ -29,11 +29,12 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
import org.apache.hadoop.hive.ql.io.AcidUtils;
import org.apache.hadoop.hive.ql.io.BucketizedHiveInputFormat;
-import org.apache.hadoop.mapred.InputFormat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hadoop.conf.Configuration;
@@ -69,10 +70,10 @@ public class SplitGrouper {
private static final Logger LOG = LoggerFactory.getLogger(SplitGrouper.class);
- // TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
- // can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private static final Cache<Path, Path> cache = Caffeine.newBuilder()
+ .maximumSize(10000000)
+ .expireAfterWrite(1, TimeUnit.DAYS)
+ .build();
private final TezMapredSplitsGrouper tezGrouper = new TezMapredSplitsGrouper();
@@ -412,8 +413,17 @@ private static MapWork populateMapWork(JobConf jobConf, String inputName) {
return work;
}
+ private static void checkAndClearCacheIfNeeded() {
+ if (cache.size() > MAX_CACHE_SIZE) {
+ cache.clear();
+ }
+ }
+
private boolean schemaEvolved(InputSplit s, InputSplit prevSplit, boolean groupAcrossFiles,
- MapWork work) throws IOException {
+ MapWork work) throws IOException {
+ // Check cache size periodically
+ checkAndClearCacheIfNeeded();
+
boolean retval = false;
Path path = ((FileSplit) s).getPath();
PartitionDesc pd = HiveFileFormatUtils.getFromPathRecursively(
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
index 20fde98c26..644ce9a3fd 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java
@@ -360,17 +360,17 @@ private static RecordUpdater getRecordUpdater(JobConf jc,
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap) throws IOException {
+ Map<Path, Path> cacheMap) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema) throws IOException {
+ Map<Path, Path> cacheMap, boolean ignoreSchema) throws IOException {
return getFromPathRecursively(pathToPartitionInfo, dir, cacheMap, ignoreSchema, false);
}
public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Path dir,
- Map<Map<Path, T>, Map<Path, T>> cacheMap, boolean ignoreSchema, boolean ifPresent)
+ Map<Path, Path> cacheMap, boolean ignoreSchema, boolean ifPresent)
throws IOException {
T part = getFromPath(pathToPartitionInfo, dir);
@@ -379,18 +379,7 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
|| (dir.toUri().getScheme() == null || dir.toUri().getScheme().trim().equals(""))
|| FileUtils.pathsContainNoScheme(pathToPartitionInfo.keySet()))) {
- Map<Path, T> newPathToPartitionInfo = null;
- if (cacheMap != null) {
- newPathToPartitionInfo = cacheMap.get(pathToPartitionInfo);
- }
-
- if (newPathToPartitionInfo == null) { // still null
- newPathToPartitionInfo = populateNewT(pathToPartitionInfo);
-
- if (cacheMap != null) {
- cacheMap.put(pathToPartitionInfo, newPathToPartitionInfo);
- }
- }
+ Map<Path, T> newPathToPartitionInfo = populateNewT(pathToPartitionInfo, cacheMap);
part = getFromPath(newPathToPartitionInfo, dir);
}
if (part != null || ifPresent) {
@@ -401,13 +390,21 @@ public static <T> T getFromPathRecursively(Map<Path, T> pathToPartitionInfo, Pat
}
}
- private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo) {
- Map<Path, T> newPathToPartitionInfo = new HashMap<>();
- for (Map.Entry<Path, T> entry: pathToPartitionInfo.entrySet()) {
- T partDesc = entry.getValue();
- Path pathOnly = Path.getPathWithoutSchemeAndAuthority(entry.getKey());
+ private static <T> Map<Path, T> populateNewT(Map<Path, T> pathToPartitionInfo,
+ Map<Path, Path> cacheMap) {
+ Map<Path, T> newPathToPartitionInfo = new HashMap<>(pathToPartitionInfo.size());
+
+ pathToPartitionInfo.forEach((originalPath, partDesc) -> {
+ Path pathOnly = cacheMap != null ?
+ cacheMap.get(originalPath) : null;
+ if (pathOnly == null) {
+ pathOnly = Path.getPathWithoutSchemeAndAuthority(originalPath);
+ if (cacheMap != null) {
+ cacheMap.put(originalPath, pathOnly);
+ }
+ }
newPathToPartitionInfo.put(pathOnly, partDesc);
- }
+ });
return newPathToPartitionInfo;
}
--
2.48.0
````
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189008451
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, how did you reproduce this in first place? Could you please share the query and tables definitions.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193987257
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
>
> * **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
> * **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
Sorry,Sir.
Because I previously reviewed the code and found that the submission and management logic of location_based_table is basically the same as that of hadoopCatalog. Therefore, I previously defaulted to treating location_based_table as HadoopCatalogTable. I confused the concepts, and this is my mistake.
Additionally, in the production environment, we currently only use HadoopTables (location_based_table) in HIVE.
(In Spark, I use hadoopCatalog to create a table. After Spark maintains this table, in HIVE I use HadoopTables to query the hadoopCatalog table maintained by Spark.)
Therefore, I only set the properties of IcebergHiveCatalog in HIVE (But I did not use HMS Catalog in the production environment, it was only used for some testing) , and did not set the related properties of other types of iceberg (including HadoopCatalog,Because HadoopTables can be used without any setup,so no need setup/use hadoopCatalog).
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193987257
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
>
> * **HadoopTables**: Implementation of Iceberg tables that uses the Hadoop FileSystem to store metadata and manifests
> * **HadoopCatalog**: provides a way to use table names like db.table to work with path-based tables under a common location. It uses a specified directory under a specified filesystem as the warehouse directory, and organizes multiple levels directories that mapped to the database, namespace and the table respectively.
Sorry,Sir.
Because I previously reviewed the code and found that the submission and management logic of location_based_table is basically the same as that of hadoopCatalog. Therefore, I previously defaulted to treating location_based_table as HadoopCatalogTable. I confused the concepts, and this is my mistake.
Additionally, in the production environment, we currently only use HadoopTables (location_based_table) in HIVE.
(In Spark, I use hadoopCatalog to create a table. After Spark maintains this table, in HIVE I use HadoopTables to query the hadoopCatalog table maintained by Spark.)
Therefore, I only set the properties of IcebergHiveCatalog in HIVE, and did not set the related properties of other types of iceberg (including HadoopCatalog,Because HadoopTables can be used without any setup.).
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189182854
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @deniskuzZ
>
> > i reverted to static as it was a global cache before
>
> So, if I do this, is it possible that it will cause splitGrouper to produce incorrect calculation results? Because no one updates it.
>
> ```
> alter table xxx drop partition([pt=11);
> alter table xxx add partition([pt=11) location '/p1';
> alter table xxx drop partition([pt=11);
> alter table xxx add partition([pt=11) location '/p2';
> alter table xxx drop partition([pt=11);
> alter table xxx add partition([pt=11) location '/p1';
> ```
i removed the global cache as i don't see how it's even being used. However, it wouldn't cause an issue since it just contains normalized path mapping and doesn't cache the `PartitionDesc`
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2189531594
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> how did you reproduce this in first place? Could you please share the query and tables definitions.
> I'm not trying to test for concurrency issues, I just want to trigger SplitGrouper in a way that uses the cache.
@deniskuzZ
Currently, my memory is a bit hazy. The necessary conditions to trigger this issue are as follows:
1.Use LLAP or a persistent Tez DAG(reuse on yarn) to read an Iceberg partition table.
2.The Iceberg table is just a regular Hadoop-catalog partitioned table.
However, this issue is not consistently reproducible. We had been using IcebergStorageHandler for a long time without encountering this problem before.
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2193107853
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, that setup uses `HadoopTables` rather than `HadoopCatalog` !
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2190553680
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
@BsoBird, thanks for the details, it helped a lot. i managed to create a test that initializes the cache from `SplitGroper`.
Note, I struggled to create an iceberg table backed by HadoopCatalog
````
set iceberg.catalog.ice01.type=hadoop;
set iceberg.catalog.ice01.warehouse=/tmp;
CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
PARTITIONED BY (p1 STRING, p2 STRING)
STORED BY ICEBERG STORED AS ORC
LOCATION '/tmp/ice01/orders'
TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
````
Caused by: java.lang.IllegalArgumentException: Cannot set a custom location for a path-based table. Expected /tmp/default/orders but got file:/tmp/ice01/orders
````
CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
PARTITIONED BY (p1 STRING, p2 STRING)
STORED BY ICEBERG STORED AS ORC
LOCATION '/tmp/ice01/orders'
TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
````
Caused by: java.lang.IllegalArgumentException: Table location not set
Wonder how were you able to create one?
--
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-27944: When HIVE-LLAP reads the ICEBERG table, a deadlock may occur. [hive]
Posted by "BsoBird (via GitHub)" <gi...@apache.org>.
BsoBird commented on code in PR #5934:
URL: https://github.com/apache/hive/pull/5934#discussion_r2191831532
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java:
##########
@@ -71,8 +70,8 @@ public class SplitGrouper {
// TODO This needs to be looked at. Map of Map to Map... Made concurrent for now since split generation
// can happen in parallel.
- private static final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
- new ConcurrentHashMap<>();
+ private final Map<Map<Path, PartitionDesc>, Map<Path, PartitionDesc>> cache =
Review Comment:
> @BsoBird, thanks for the details, it helped a lot. i managed to create a test that initializes the cache from `SplitGroper`. Note, I struggled to create an iceberg table backed by HadoopCatalog
>
> ```
> set iceberg.catalog.ice01.type=hadoop;
> set iceberg.catalog.ice01.warehouse=/tmp;
>
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Cannot set a custom location for a path-based table. Expected /tmp/default/orders but got file:/tmp/ice01/orders
>
> ```
> CREATE EXTERNAL TABLE orders (orderid INT, quantity INT, itemid INT, tradets TIMESTAMP)
> PARTITIONED BY (p1 STRING, p2 STRING)
> STORED BY ICEBERG STORED AS ORC
> LOCATION '/tmp/ice01/orders'
> TBLPROPERTIES('format-version'='2', 'iceberg.catalog'='ice01');
> ```
>
> Caused by: java.lang.IllegalArgumentException: Table location not set
>
> Wonder how were you able to create one?
@deniskuzZ
Example:
```
----spark3.4.1+iceberg 1.4.2
CREATE TABLE IF NOT EXISTS datacenter.test.test_data_02 (
id string,name string
)
PARTITIONED BY (name,bucket(16,id))
TBLPROPERTIES ('read.orc.vectorization.enabled'='true','write.format.default'='orc','write.orc.bloom.filter.columns'='id','write.orc.compression-codec'='zstd','write.metadata.previous-versions-max'='3','write.metadata.delete-after-commit.enabled'='true')
STORED AS iceberg;
insert into datacenter.test.test_data_02(id,name) values('1','a'),('2','b');
--hive
CREATE EXTERNAL TABLE iceberg_dwd.test_data_02
STORED BY 'org.apache.iceberg.mr.hive.HiveIcebergStorageHandler'
LOCATION 'hdfs://xxxxxx/iceberg-catalog/warehouse/test/test_data_02'
TBLPROPERTIES ('iceberg.catalog'='location_based_table','engine.hive.enabled'='true');
```
But in reality, using Iceberg tables from other catalogs in HIVE is not always smooth. For example, if the table's location is set only using Iceberg properties without setting the HIVE table's LOCATION property, there's a high chance the table won't function properly.
--
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