Re: [PR] HIVE-28578: Concurrency issue in updateTableColumnStatistics [hive]

Thu, 13 Nov 2025 02:04:11 -0800 


deniskuzZ commented on PR #6159:
URL: https://github.com/apache/hive/pull/6159#issuecomment-3526906611

   > I tried the similar update on MySQL, the black transaction is waiting 
until "Lock wait timeout exceeded", <img alt="Screenshot 2025-11-13 at 09 06 
43" width="1269" height="239" 
src="https://private-user-images.githubusercontent.com/4394480/513614719-4db53c7b-c492-4ff5-a2a0-22a1ee0717e1.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NjMwMjYzMjQsIm5iZiI6MTc2MzAyNjAyNCwicGF0aCI6Ii80Mzk0NDgwLzUxMzYxNDcxOS00ZGI1M2M3Yi1jNDkyLTRmZjUtYTJhMC0yMmExZWUwNzE3ZTEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MTExMyUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTExMTNUMDkyNzA0WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9ZDA0MmM3ZTU5ZmI3NjBkNTA1YmM5MTM0YTJlOTQ2ZmUzMzNhNWFhMjM2YTA5OTkzZGQ3YTkyZGQ1ZGQ1NTUyZiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.8D8zMvzRBih7ICWFKLJ1PoZCBMtjZRSsotsPncNO_LY";>
   
   that is 100% true, however, MVCC is better because:
   - Faster claim (1ms vs 10ms)
   - Parallel claim attempts (database resolves conflicts) 
   - Simpler code (no savepoints) Database uses MVCC to serialize at commit time
   - No locks held during work phase
   PS: we already use MVCC in ObjectStore: updateParameterWithExpectedValue()
   
   updated patch:
   ````
   Subject: [PATCH] DRAFT
   ---
   Index: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
   IDEA additional info:
   Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
   <+>UTF-8
   ===================================================================
   diff --git 
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
   --- 
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
  (revision c729ea19807c0c0ca6f1df4870fff49660e95a85)
   +++ 
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
  (date 1763028067727)
   @@ -9171,21 +9171,39 @@
          int maxRetries = MetastoreConf.getIntVar(conf, 
ConfVars.METASTORE_S4U_NOWAIT_MAX_RETRIES);
          long sleepInterval = MetastoreConf.getTimeVar(conf,
              ConfVars.METASTORE_S4U_NOWAIT_RETRY_SLEEP_INTERVAL, 
TimeUnit.MILLISECONDS);
   +      
   +      final String versionParamKey = "hive.metastore.table.version";
   +      
          Map<String, String> result = new RetryingExecutor<>(maxRetries, () -> 
{
   -        Ref<Exception> exceptionRef = new Ref<>();
   -        String savePoint = "uts_" + 
ThreadLocalRandom.current().nextInt(10000) + "_" + System.nanoTime();
   -        setTransactionSavePoint(savePoint);
   -        executePlainSQL(
   -            sqlGenerator.addForUpdateNoWait("SELECT \"TBL_ID\" FROM 
\"TBLS\" WHERE \"TBL_ID\" = " + mTable.getId()),
   -            exception -> {
   -              rollbackTransactionToSavePoint(savePoint);
   -              exceptionRef.t = exception;
   -            });
   -        if (exceptionRef.t != null) {
   -          throw new RetryingExecutor.RetryException(exceptionRef.t);
   -        }
            pm.refresh(mTable);
            Table table = convertToTable(mTable);
   +        String dbname = table.getDbName();
   +        String name = table.getTableName();
   +        
   +        // ✅ STEP 1: Read current version snapshot from TABLE_PARAMS
   +        String expectedVersionStr = 
table.getParameters().get(versionParamKey);
   +        if (expectedVersionStr == null) {
   +          expectedVersionStr = "0";
   +        }
   +        long newVersion = Long.parseLong(expectedVersionStr) + 1;
   +        String newVersionStr = String.valueOf(newVersion);
   +        
   +        // ✅ STEP 2: Atomically claim the version using existing MVCC API
   +        // This uses UPDATE with WHERE clause to check snapshot hasn't 
changed
   +        long affectedRows = updateParameterWithExpectedValue(table, 
versionParamKey, expectedVersionStr, newVersionStr);
   +        
   +        if (affectedRows != 1) {
   +          // Version conflict - PARAM_VALUE changed since we read it 
(concurrent modification)
   +          LOG.debug("Table {}.{} version conflict (expected={}), 
retrying...", dbname, name, expectedVersionStr);
   +          throw new RetryingExecutor.RetryException(
   +              new MetaException("The table has been modified. The parameter 
value for key '" + 
   +                  versionParamKey + "' is different"));
   +        }
   +        
   +        // ✅ STEP 3: Successfully claimed version - now do the work
   +        LOG.debug("Claimed table {}.{} version {} -> {}, proceeding with 
stats update", 
   +            dbname, name, expectedVersionStr, newVersion);
   +        
            List<String> colNames = new ArrayList<>();
            for (ColumnStatisticsObj statsObj : statsObjs) {
              colNames.add(statsObj.getColName());
   @@ -9201,17 +9219,14 @@
              MTableColumnStatistics mStatsObj = 
StatObjectConverter.convertToMTableColumnStatistics(mTable, statsDesc,
                  statsObj, colStats.getEngine());
              writeMTableColumnStatistics(table, mStatsObj, 
oldStats.get(statsObj.getColName()));
   -          // There is no need to add colname again, otherwise we will get 
duplicate colNames.
            }
    
            // TODO: (HIVE-20109) ideally the col stats stats should be in 
colstats, not in the table!
            // Set the table properties
   -        // No need to check again if it exists.
   -        String dbname = table.getDbName();
   -        String name = table.getTableName();
            MTable oldt = mTable;
            Map<String, String> newParams = new 
HashMap<>(table.getParameters());
            StatsSetupConst.setColumnStatsState(newParams, colNames);
   +        
            boolean isTxn = TxnUtils.isTransactionalTable(oldt.getParameters());
            if (isTxn) {
              if (!areTxnStatsSupported) {
   @@ -9230,7 +9245,11 @@
                oldt.setWriteId(writeId);
              }
            }
   +        
   +        // ✅ STEP 4: Add the new version to params (already updated in DB 
via directSql)
   +        newParams.put(versionParamKey, newVersionStr);
            oldt.setParameters(newParams);
   +
            return newParams;
          }).onRetry(e -> e instanceof RetryingExecutor.RetryException)
            
.commandName("updateTableColumnStatistics").sleepInterval(sleepInterval, 
interval ->
   
   ````


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to