HIVE-27224: Enhance drop table/partition command #5851

dengzhhu653 · 2025-06-10T00:19:41Z

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?

sonarqubecloud · 2025-06-18T10:37:31Z

Quality Gate passed

Issues
18 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

...store/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java

saihemanth-cloudera · 2026-01-02T20:16:50Z

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

          } catch (InvalidOperationException e) {
            // This means there are tables of something in the database
            throw new InvalidOperationException("There are still objects in the default " +
                "database for catalog " + catName);


nit: should we also throw stack trace?

IMO the message can tell the enough about the exception, we don't need to pollute the log file

sonarqubecloud · 2026-01-04T09:16:43Z

Quality Gate passed

Issues
52 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

wecharyu · 2026-01-03T10:34:49Z

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

-  public void drop_database_req(final DropDatabaseRequest req)
+  public AsyncOperationResp drop_database_req(final DropDatabaseRequest req)
      throws NoSuchObjectException, InvalidOperationException, MetaException {
    startFunction("drop_database", ": " + req.getName());


If the operation is async, the recorded API time will be not the operation finished cost time, should we record it in the handler?

nice catch, will try the timing in handler

wecharyu · 2026-01-03T16:05:52Z

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

-      LOG.warn("Error from isWritable", ex);
-      throw new MetaException("Table partition not deleted since " + dir.getParent()
-          + " access cannot be checked: " + ex.getMessage());
+      return resp;


If the returned partitions of resp is empty, the client can not know if it's finished or still running.

for add partitions, it's not asynchronous, this method wouldn't return until the handler finishes the job,

wecharyu · 2026-01-03T16:28:59Z

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

+        part_vals = getPartValsFromName(t, dropPartitionReq.getPartName());
      }
+      partNames.add(Warehouse.makePartName(t.getPartitionKeys(), part_vals));


Suggested change

part_vals = getPartValsFromName(t, dropPartitionReq.getPartName());

}

partNames.add(Warehouse.makePartName(t.getPartitionKeys(), part_vals));

partNames.add(dropPartitionReq.getPartName());

} else {

partNames.add(Warehouse.makePartName(t.getPartitionKeys(), part_vals));

}

...lone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java

wecharyu · 2026-01-03T17:00:06Z

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

+      AbstractOperationHandler<DropDatabaseRequest, DropDatabaseHandler.DropDatabaseResult> dropDatabaseOp =
+          AbstractOperationHandler.offer(this, req);
+      AbstractOperationHandler.OperationStatus status = dropDatabaseOp.getOperationStatus();
+      if (status.isFinished() && dropDatabaseOp.getResult() != null && dropDatabaseOp.getResult().isSuccess()) {


dropDatabaseOp.getResult() != null is redundant and can be removed.

if the handler is canceled, the dropDatabaseOp.getResult() might be null, will add the comment

wecharyu · 2026-01-03T17:08:15Z

...store-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java

+      // a copy is required to allow incremental replication to work correctly.
+      if (func.getResourceUris() != null && !func.getResourceUris().isEmpty()) {
+        for (ResourceUri uri : func.getResourceUris()) {
+          if (uri.getUri().toLowerCase().startsWith("hdfs:") && needsCm) {


This function can fast-return if needsCm is false.

nice catch, will do

wecharyu · 2026-01-04T03:24:00Z

...-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AbstractOperationHandler.java

+  private A result;
+  private boolean async;
+  private Future<A> future;
+  private ExecutorService executor;


Is it better to use a shared thread pool for the operation handler? In the current implementation, the number of threads is not bounded, which could lead to resource exhaustion or even crashes.

The number of handler's threads is limited by the max threads the thrift server can spawn, which is set by hive.metastore.server.max.threads.

In production, I think we shouldn't not have such a high drop databases/table operations happens near the same time, usually the database is the bottleneck before the Metastore hits the limit, if this is the case, we can tune down the hive.metastore.server.max.threads.

In the async mode, a thread may trigger multiple operation handlers, so the hive.metastore.server.max.threads could not limit total threads here. If we configure a fixed size pool for the async operations, it can help limit service traffic to some extent.

Usually the new handler runs inside the same thread as the parent, such as DropDatabaseHandler, involves multiple DropTableHandlers, these DropTableHandlers runs inside the same thread as the DropDatabaseHandler

Usually the new handler runs inside the same thread as the parent, such as DropDatabaseHandler, involves multiple DropTableHandlers, these DropTableHandlers run inside the same thread as the DropDatabaseHandler

Oh I mean that for async request, the HMSHandler thread could create an operation handler where the executor starts a new thread. Then the HMSHandler thread finish this request immediatly and could handle another request where it may produce another new thread.

If such async request is frequent, it may lead to an explosion in the number of threads.

Though Metastore handles the async request in background, the client doesn't, it pings the server for the status until the end:

hive/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/ThriftHiveMetaStoreClient.java

Lines 1524 to 1529 in 79f63b6

while (!resp.isFinished() && !Thread.currentThread().isInterrupted()) {

resp = client.drop_database_req(req);

if (resp.getMessage() != null) {

LOG.info(resp.getMessage());

}

}

the client will know whether the request is successful or not as usual at the end, and the Metastore needs a handler thread to answer the request.

the HMSHandler thread finish this request immediatly

hive/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AbstractOperationHandler.java

Line 179 in 79f63b6

result = async ? future.get(timeout, TimeUnit.MILLISECONDS) : future.get();

Now it will wait for 5 seconds before answering the API for long running drop unless the request is satisfied within this timeout, then we can get the result.

There are customized clients based on ThriftHiveMetastore.Iface, which may not guarantee such behavior.

asyncDrop defaults to false, unless it's specified explicitly as true, then the customized client should take care of his case.

wecharyu · 2026-01-04T03:51:52Z

...store-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java

+        dropRequest.setEnvContext(context);
+        // Drop the table but not its data
+        dropRequest.setDeleteData(false);
+        dropRequest.setDropPartitions(true);


Better to explicitly disable asyncDrop here:

dropRequest.setAsyncDrop(false);

wecharyu · 2026-01-04T11:08:00Z

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

+      AbstractOperationHandler<DropPartitionsRequest, DropPartitionsHandler.DropPartitionsResult> dropPartsOp =
+          AbstractOperationHandler.offer(this, request);


Such calls can be changed as following, including other related api:

Suggested change

AbstractOperationHandler<DropPartitionsRequest, DropPartitionsHandler.DropPartitionsResult> dropPartsOp =

AbstractOperationHandler.offer(this, request);

DropPartitionsHandler dropPartsOp = (DropPartitionsHandler) AbstractOperationHandler.offer(this, request);

saihemanth-cloudera · 2026-01-05T17:39:43Z

A couple of test failures seem to be related to this patch.

wecharyu · 2026-01-06T04:41:49Z

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

+          wh.addToChangeManagement(funcCmPath);
+        }
+        if (req.isDeleteData()) {
+          // Moving the data deletion out of the async handler.


I think we should move this into the operation handler, because if a thrift client only calls this api once in async mode, then such cleanup code would never be run.

In async mode, the client still need to ping the server for the operation status until the end, the client needs to know whether the request is a failure or not.
The main reason is the TUGIBasedProcessor/TUGIAssumingProcessor might close the shared FileSystem behind, causing the "java.io.IOException: Filesystem closed" for the handler running in background.

Still we need to address this "Filesystem closed" issue, as we don't know whether there are Filesystem operations in the Metastore listeners.

FileSystem.closeAllForUGI(clientUgi); in TUGIAssumingProcessor seems a bug, assume that there are two requests with same ugi to handle the same path uri concurrently, it may also hit the "Filesystem closed" issue.

This is indeed a tricky problem, not sure if we can only remove cache for inactive ugi to solve it. And for this thread, it still has an issue if the client crush between two pings before the operation handler finished, the cleanup code will not take effect either.

wecharyu · 2026-01-06T05:08:34Z

...-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AbstractOperationHandler.java

+            return execute();
+          } finally {
+            afterExecute();
+            OPID_CLEANER.schedule(() -> OPID_TO_HANDLER.remove(id), 1, TimeUnit.HOURS);


Should we consider adding a OPID_TO_STATUS? Because a thrift client should be able to get the result of an operation id even if the operation is finished, but now if the operation finished all things are removed.

the result is stored at the future,

hive/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AbstractOperationHandler.java

Line 179 in 79f63b6

result = async ? future.get(timeout, TimeUnit.MILLISECONDS) : future.get();

the status can be determined by the status of this future, which would be cleared 1 hours later

asf-ci-hive added tests pending tests failed and removed tests pending tests failed labels Jun 10, 2025

dengzhhu653 force-pushed the HIVE-27224 branch from 02a1e6a to 148a424 Compare June 11, 2025 08:35

asf-ci-hive added tests unstable tests pending tests failed and removed tests pending tests unstable tests failed labels Jun 11, 2025

dengzhhu653 changed the title ~~[WIP] HIVE-27224: Enhance drop table/partition command~~ HIVE-27224: Enhance drop table/partition command Jun 12, 2025

asf-ci-hive added tests passed tests pending and removed tests pending tests passed labels Jun 12, 2025

asf-ci-hive added tests passed and removed tests pending labels Jun 18, 2025

minor refactor

e32a2bf

asf-ci-hive added tests pending tests unstable and removed tests passed tests pending labels Dec 30, 2025

fix NPE

459352f

asf-ci-hive added tests pending tests passed and removed tests unstable tests pending labels Dec 30, 2025

dengzhhu653 requested review from saihemanth-cloudera and wecharyu December 31, 2025 00:26

add partitions req

dba77b7

saihemanth-cloudera reviewed Jan 2, 2026

View reviewed changes

...store/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java Show resolved Hide resolved

saihemanth-cloudera reviewed Jan 2, 2026

View reviewed changes

normalize

3e7b326

asf-ci-hive added tests pending tests unstable and removed tests passed tests pending labels Jan 4, 2026

minor and fix ut

79f63b6

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

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

wecharyu reviewed Jan 4, 2026

View reviewed changes

wecharyu reviewed Jan 6, 2026

View reviewed changes

	while (!resp.isFinished() && !Thread.currentThread().isInterrupted()) {
	resp = client.drop_database_req(req);
	if (resp.getMessage() != null) {
	LOG.info(resp.getMessage());
	}
	}

		AbstractOperationHandler<DropPartitionsRequest, DropPartitionsHandler.DropPartitionsResult> dropPartsOp =
		AbstractOperationHandler.offer(this, request);

	AbstractOperationHandler<DropPartitionsRequest, DropPartitionsHandler.DropPartitionsResult> dropPartsOp =
	AbstractOperationHandler.offer(this, request);
	DropPartitionsHandler dropPartsOp = (DropPartitionsHandler) AbstractOperationHandler.offer(this, request);

HIVE-27224: Enhance drop table/partition command #5851

Are you sure you want to change the base?

HIVE-27224: Enhance drop table/partition command #5851

Conversation

dengzhhu653 commented Jun 10, 2025

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!

sonarqubecloud bot commented Jun 18, 2025

Quality Gate passed

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 4, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dengzhhu653 Jan 5, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dengzhhu653 Jan 6, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saihemanth-cloudera commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dengzhhu653 Jan 6, 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!

Choose a reason for hiding this comment

dengzhhu653 Jan 5, 2026 •

edited

Loading

dengzhhu653 Jan 6, 2026 •

edited

Loading

dengzhhu653 Jan 6, 2026 •

edited

Loading