HIVE-29437: Iceberg: Fix concurrency issues between compaction and co… #6292

difin · 2026-02-03T18:33:07Z

…ncurrent write operations.

What changes were proposed in this pull request?

Fixing concurrency issues between compaction and concurrent write operations.

Why are the changes needed?

It was found in downstream testing that when Hive Iceberg compaction is running in parallel to Spark write operations on the same table, compaction sometimes produces wrong results. Before committing, when Hive already has the compacted data files that need to replace existing, uncompacted data and delete files in a table or partition, it collects those uncompacted data and delete files to replace them with the compacted files. The issue is that Hive collects those uncompacted data and delete files from the latest Iceberg snapshot instead of using the original snapshot. The latest snapshot may contain different data because of concurrent write operations, which can lead to data corruption.

Does this PR introduce any user-facing change?

No

How was this patch tested?

The fix was validated downstream with concurrent Spark write operations and Hive Iceberg compaction.

...erg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java

...eberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergCompactionUtil.java

deniskuzZ · 2026-02-09T08:52:31Z

iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java

+   * This is needed because hiveConf() returns the original conf passed to start(),
+   * which may not have the connection URL that was set in the handler's serverConf.
+   */
+  public String getConnectionURL() {


is this the code used to set the DB url?

private static void setupMetastoreDB(String dbURL) throws Exception { HiveConf conf = new HiveConf(); MetastoreConf.setVar(conf, MetastoreConf.ConfVars.CONNECT_URL_KEY, "jdbc:derby:" + DERBY_PATH + ";create=true"); TestTxnDbUtil.prepDb(conf); }

why we use baseHandler.getConf()?

could we reuse MetaStoreInit.getConnectionURL(Configuration conf)

could we reuse MetaStoreInit.getConnectionURL(Configuration conf)

No, MetaStoreInit.getConnectionURL(Configuration conf) returns default db url - Derby in-memory:

jdbc:derby:memory:/Users/dfingerman/workspace/hive-upstream-difin/iceberg/iceberg-handler/target/tmp/junit_metastore_db;create=true

static String getConnectionURL(Configuration conf) { return MetastoreConf.getVar(conf, ConfVars.CONNECT_URL_KEY, ""); }

Current method returns the correct file-based derby db url:
shell.metastore().getConnectionURL():

jdbc:derby:/var/folders/ks/4pwh80t957gc8q2z7jdblpw80000gq/T/hive3437069780704791128/metastore_db;create=true

This is what took me 1-2 days to find why compaction command wasn't able to resolve correct database during compaction command analysis.

are we using fallback here?
// Fallback: construct from DERBY_PATH pattern
return "jdbc:derby:" + DERBY_PATH + ";create=true";

someone should be setting the CONNECT_URL_KEY, otherwise, how everything else works?
is this the place?

private static void setupMetastoreDB(String dbURL) throws Exception { HiveConf conf = new HiveConf(); MetastoreConf.setVar(conf, MetastoreConf.ConfVars.CONNECT_URL_KEY, "jdbc:derby:" + DERBY_PATH + ";create=true"); TestTxnDbUtil.prepDb(conf); }

why not move to initConf

MetastoreConf.setVar(conf, MetastoreConf.ConfVars.CONNECT_URL_KEY, "jdbc:derby:" + DERBY_PATH + ";create=true");

why not move to initConf

MetastoreConf.setVar(conf, MetastoreConf.ConfVars.CONNECT_URL_KEY, "jdbc:derby:" + DERBY_PATH + ";create=true");

It doesn't work - when compaction command is being analyzed it connects to the default derby in-memory db.
It only works when this is added in HiveIcebergStorageHandlerWithEngineBase#executeConcurrently

shell.setHiveSessionValue(HiveConf.ConfVars.METASTORE_CONNECT_URL_KEY.varname, shell.metastore().getConnectionURL());

ok, but what about metastore().getConnectionURL() do we always go with fallback? can we set CONNECT_URL_KEY in init and then return in getConnectionURL()?

...eberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergTableOptimizer.java

deniskuzZ · 2026-02-09T10:54:38Z

.../src/test/java/org/apache/iceberg/mr/hive/test/utils/HiveIcebergStorageHandlerTestUtils.java

    shell.setHiveConfValue("tez.counters.max", "1024");
+
+    // Settings for Hive Iceberg Compaction
+    shell.setHiveConfValue(HiveConf.ConfVars.HIVE_LOCK_MANAGER.varname,


do not set configs that are already defaults

HiveConf:

HIVE_LOCK_MANAGER("hive.lock.manager", "org.apache.hadoop.hive.ql.lockmgr.zookeeper.ZooKeeperHiveLockManager", ""),

Where is this defined as default?

that is not good actually, DbLockManager should be the default. Isn't ACID default upstream?

.../src/test/java/org/apache/iceberg/mr/hive/test/utils/HiveIcebergStorageHandlerTestUtils.java

...andler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerWithEngineBase.java

deniskuzZ · 2026-02-09T10:58:34Z

...andler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerWithEngineBase.java

+            Configuration shellConf = shell.getHiveConf();
+
+            if (metastoreConnectUrl != null) {
+              shellConf.set(HiveConf.ConfVars.METASTORE_CONNECT_URL_KEY.varname, metastoreConnectUrl);


i am not convinced we should be doing this. can't we set globally METASTORE_CONNECT_URL_KEY?
like System.setProperty(HiveConf.ConfVars.METASTORE_CONNECT_URL_KEY.varname,
MetastoreConf.getVar(hiveConf, MetastoreConf.ConfVars.CONNECT_URL_KEY));

deniskuzZ · 2026-02-09T11:02:20Z

iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestConflictingDataFiles.java

+
+    String[] sql = new String[] {
+        "INSERT INTO ice_t SELECT i*100, p*100 FROM ice_t",
+        "ALTER TABLE ice_t compact 'MAJOR' and wait"


i don't think that is a proper test.
ALTER TABLE ice_t compact 'MAJOR' would initiate new IOW query, would it synchronize with INSERT? HiveIcebergStorageHandlerStub#waitForAllWritesToComplete

please check above, maybe we need to execute IOW with compaction session attributes.
or fix the synchronization

deniskuzZ

+1

please create new JIRA for the test part

…ncurrent write operations.

sonarqubecloud · 2026-02-10T16:31:56Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

asf-ci-hive added the tests pending label Feb 3, 2026

difin requested a review from deniskuzZ February 3, 2026 18:47

deniskuzZ reviewed Feb 3, 2026

View reviewed changes

...erg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java Show resolved Hide resolved

deniskuzZ reviewed Feb 3, 2026

View reviewed changes

...eberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergCompactionUtil.java Show resolved Hide resolved

asf-ci-hive added tests passed tests pending tests failed and removed tests pending tests passed labels Feb 3, 2026

difin force-pushed the iceberg_compaction_concurrency_fix branch from b842441 to 5433c78 Compare February 4, 2026 21:04

asf-ci-hive added tests pending tests unstable and removed tests failed tests pending labels Feb 4, 2026

difin force-pushed the iceberg_compaction_concurrency_fix branch from 5433c78 to 508798e Compare February 5, 2026 21:17

asf-ci-hive added tests pending tests unstable and removed tests unstable tests pending labels Feb 5, 2026

difin force-pushed the iceberg_compaction_concurrency_fix branch from 508798e to 22e610b Compare February 7, 2026 00:35

asf-ci-hive added tests pending tests passed and removed tests unstable tests pending labels Feb 7, 2026

deniskuzZ reviewed Feb 9, 2026

View reviewed changes

...eberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergTableOptimizer.java Outdated Show resolved Hide resolved

deniskuzZ reviewed Feb 9, 2026

View reviewed changes

.../src/test/java/org/apache/iceberg/mr/hive/test/utils/HiveIcebergStorageHandlerTestUtils.java Outdated Show resolved Hide resolved

deniskuzZ reviewed Feb 9, 2026

View reviewed changes

...andler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerWithEngineBase.java Outdated Show resolved Hide resolved

deniskuzZ reviewed Feb 9, 2026

View reviewed changes

difin force-pushed the iceberg_compaction_concurrency_fix branch from 22e610b to 2e0825d Compare February 10, 2026 02:17

asf-ci-hive added tests pending tests unstable and removed tests passed tests pending labels Feb 10, 2026

deniskuzZ approved these changes Feb 10, 2026

View reviewed changes

asf-ci-hive added tests pending tests unstable and removed tests unstable tests pending labels Feb 10, 2026

HIVE-29437: Iceberg: Fix concurrency issues between compaction and co…

3bf65e2

…ncurrent write operations.

difin force-pushed the iceberg_compaction_concurrency_fix branch from 2e0825d to 3bf65e2 Compare February 10, 2026 15:14

asf-ci-hive added tests pending and removed tests unstable labels Feb 10, 2026

asf-ci-hive added tests passed and removed tests pending labels Feb 10, 2026

difin merged commit 39b9ef1 into apache:master Feb 10, 2026
2 checks passed

HIVE-29437: Iceberg: Fix concurrency issues between compaction and co… #6292

HIVE-29437: Iceberg: Fix concurrency issues between compaction and co… #6292

Conversation

difin commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

Uh oh!

deniskuzZ Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

difin Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

difin Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deniskuzZ Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

difin Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

deniskuzZ Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deniskuzZ left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 10, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

difin commented Feb 3, 2026 •

edited

Loading

deniskuzZ Feb 9, 2026 •

edited

Loading

difin Feb 9, 2026 •

edited

Loading

deniskuzZ Feb 9, 2026 •

edited

Loading

difin Feb 9, 2026 •

edited

Loading

deniskuzZ Feb 9, 2026 •

edited

Loading

deniskuzZ Feb 9, 2026 •

edited

Loading