HIVE-27150: Drop single partition can also support direct sql #4123

wecharyu · 2023-03-19T09:49:53Z

What changes were proposed in this pull request?

We want to reuse the dropPartitionsInternal() method for drop_partition_common api to enjoy the high performance, to achieve this we will refactor the RawStore#dropPartition api.

Why are the changes needed?

support direct sql for drop single partition
reduce one query to the DB, that is MPartition query in ObjectStore#dropPartition

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass all existing tests.
Add the benchmark test for dropPartition:

java -jar hmsbench-jar-with-dependencies.jar -H localhost --savedata /tmp/benchdata --sanitize -N 10 -N 100 -o bench_results.csv -C -d testbench --params=100 -E 'list.*' -E 'createTable' -E 'dropTable.*' -E 'get.*' -E 'add.*' -E 'renameTable.*' -E 'dropDatabase.*' -E 'openTxn.*' -E 'PartitionManagementTask'

Before use direct sql:

Operation                      Mean     Med      Min      Max      Err%
dropPartition                  29.60    28.32    27.38    57.81    13.86
dropPartition.10               263.1    272.3    160.7    292.5    12.09
dropPartition.100              2461     2735     1610     4381     22.36
dropPartitions.10              22.97    22.46    21.56    36.05    8.306
dropPartitions.100             26.97    26.35    24.71    35.25    7.569

After use direct sql:

Operation                      Mean     Med      Min      Max      Err%
dropPartition                  24.76    24.56    20.86    36.23    11.47
dropPartition.10               190.1    165.7    153.5    275.1    23.76
dropPartition.100              2181     2591     1482     4831     28.61
dropPartitions.10              17.71    17.43    15.84    25.37    7.857
dropPartitions.100             34.42    33.10    31.61    58.97    11.98

wecharyu · 2023-03-24T15:43:53Z

@deniskuzZ @kasakrisz @saihemanth-cloudera: Could you please review this PR?

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

...e-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java

wecharyu · 2023-03-28T18:15:33Z

Put the benchmark in PR description, it shows some performance improvement. FYI: @deniskuzZ @saihemanth-cloudera @VenuReddy2103

saihemanth-cloudera · 2023-03-31T10:18:50Z

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

-      if (!ms.dropPartition(catName, db_name, tbl_name, part_vals)) {
+      String partName = Warehouse.makePartName(tbl.getPartitionKeys(), part_vals);
+      if (!ms.dropPartition(catName, db_name, tbl_name, partName)) {
        throw new MetaException("Unable to drop partition");


Should we throw the exception stack trace/reason for failure?

ms.dropPartition() will throw the exception, we do not need to throw this MetaException here, will remove this code.

saihemanth-cloudera · 2023-03-31T10:49:33Z

...tastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java

    Assert.assertEquals(2, numPartitions);

    try (AutoCloseable c = deadline()) {
-      objectStore.dropPartition(DEFAULT_CATALOG_NAME, DB1, TABLE1, value1);


Ideally, I would like to keep the old tests and add new tests that takes in "partName" as argument.

saihemanth-cloudera · 2023-03-31T11:02:29Z

...metastore-benchmarks/src/main/java/org/apache/hadoop/hive/metastore/tools/HMSBenchmarks.java

+            return null;
+          }),
          null);
-    } catch (TException e) {


Why do we need to remove this? I think we are somehow the percentage of error records is not correct.

I followed the benchmarkDropPartitions() and some others, catching exception here should have nothing to do with benchmark statistics, because the result statistic is in measure() method:

hive/standalone-metastore/metastore-tools/tools-common/src/main/java/org/apache/hadoop/hive/metastore/tools/MicroBenchmark.java

Lines 84 to 87 in 745dbf7

public DescriptiveStatistics measure(@Nullable Runnable pre,

@NotNull Runnable test,

@Nullable Runnable post) {

// Warmup phase

Also the Err% should be Coefficient of variation of successful operations rather than percentage of error records:

hive/standalone-metastore/metastore-tools/tools-common/src/main/java/org/apache/hadoop/hive/metastore/tools/BenchmarkSuite.java

Line 207 in 745dbf7

double err = stats.getStandardDeviation() / mean * 100;

saihemanth-cloudera

Can you consider the given suggestions?

wecharyu · 2023-03-31T17:44:06Z

Address comments. cc: @saihemanth-cloudera

sonarqubecloud · 2023-03-31T21:02:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
7 Code Smells

No Coverage information
No Duplication information

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

...tastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java

saihemanth-cloudera

Can you look at the given suggestions?

saihemanth-cloudera

LGTM +1

deniskuzZ

LGTM +1

…eviewed by Sai Hemanth Gantasala, Denys Kuzmenko) Closes apache#4123

kgyrtkirk added tests pending tests failed tests passed and removed tests pending tests failed labels Mar 19, 2023

deniskuzZ reviewed Mar 24, 2023

View reviewed changes

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

kgyrtkirk added tests pending tests passed and removed tests passed tests pending labels Mar 25, 2023

saihemanth-cloudera reviewed Mar 27, 2023

View reviewed changes

...e-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Show resolved Hide resolved

wecharyu added 4 commits March 27, 2023 23:20

HIVE-27150: Drop single partition can also support direct sql

4936d2d

fix itests compile

13ca31e

deprecate the old dropPartition api

39b201c

support drop multiple single partition for benchmark test

9283546

wecharyu force-pushed the HIVE-27150 branch from 586cba5 to 9283546 Compare March 28, 2023 18:09

kgyrtkirk added tests pending and removed tests passed labels Mar 28, 2023

kgyrtkirk added tests passed and removed tests pending labels Mar 28, 2023

saihemanth-cloudera reviewed Mar 31, 2023

View reviewed changes

saihemanth-cloudera requested changes Mar 31, 2023

View reviewed changes

add new dropPartition test

c52df72

kgyrtkirk removed the tests passed label Mar 31, 2023

kgyrtkirk added the tests pending label Mar 31, 2023

kgyrtkirk added tests passed and removed tests pending labels Mar 31, 2023

saihemanth-cloudera reviewed Apr 10, 2023

View reviewed changes

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

saihemanth-cloudera reviewed Apr 10, 2023

View reviewed changes

...tastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java Show resolved Hide resolved

saihemanth-cloudera requested changes Apr 10, 2023

View reviewed changes

saihemanth-cloudera approved these changes Apr 14, 2023

View reviewed changes

deniskuzZ approved these changes Apr 24, 2023

View reviewed changes

deniskuzZ merged commit 7f46b40 into apache:master Apr 24, 2023

henrib pushed a commit to henrib/hive that referenced this pull request Apr 25, 2023

HIVE-27150: Support direct sql in drop single partition (Wechar Yu, r…

dbbe9a3

…eviewed by Sai Hemanth Gantasala, Denys Kuzmenko) Closes apache#4123

yeahyung pushed a commit to yeahyung/hive that referenced this pull request Jul 20, 2023

HIVE-27150: Support direct sql in drop single partition (Wechar Yu, r…

5161356

…eviewed by Sai Hemanth Gantasala, Denys Kuzmenko) Closes apache#4123

tarak271 pushed a commit to tarak271/hive-1 that referenced this pull request Dec 19, 2023

HIVE-27150: Support direct sql in drop single partition (Wechar Yu, r…

84fb10d

…eviewed by Sai Hemanth Gantasala, Denys Kuzmenko) Closes apache#4123

	public DescriptiveStatistics measure(@Nullable Runnable pre,
	@NotNull Runnable test,
	@Nullable Runnable post) {
	// Warmup phase

HIVE-27150: Drop single partition can also support direct sql #4123

HIVE-27150: Drop single partition can also support direct sql #4123

Uh oh!

Conversation

wecharyu commented Mar 19, 2023 • 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!

wecharyu commented Mar 24, 2023

Uh oh!

Uh oh!

Uh oh!

wecharyu commented Mar 28, 2023

Uh oh!

saihemanth-cloudera Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

wecharyu Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

saihemanth-cloudera Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

saihemanth-cloudera Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

wecharyu Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

saihemanth-cloudera left a comment

Choose a reason for hiding this comment

Uh oh!

wecharyu commented Mar 31, 2023

Uh oh!

sonarqubecloud bot commented Mar 31, 2023

Uh oh!

Uh oh!

Uh oh!

saihemanth-cloudera left a comment

Choose a reason for hiding this comment

Uh oh!

saihemanth-cloudera left a comment

Choose a reason for hiding this comment

Uh oh!

deniskuzZ left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wecharyu commented Mar 19, 2023 •

edited

Loading