HIVE-27050: Restrict reducer extrapolation to contain number of small… by difin · Pull Request #4477 · apache/hive

difin · 2023-07-11T14:15:12Z

… files being created for Iceberg tables.

What changes were proposed in this pull request?

Setting "hive.tez.max.partition.factor" to "1.0" for Iceberg tables.

Why are the changes needed?

When Iceberg MOR tables are involved in update/delete/merges, Hive estimates the number of reducers as "1". But due to "hive.tez.max.partition.factor" which defaults to "2.0", it will double the number of reducers.
It creates very small positional delete files spreading across different reducers. This will cause problems during reading, as all files should be opened for reading.

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

Passing pre-commit tests.

aturoczy

+1

okumin · 2023-07-12T12:52:52Z

    // TODO: remove once we have both Fanout and ClusteredWriter available: HIVE-25948
    HiveConf.setIntVar(configuration, HiveConf.ConfVars.HIVEOPTSORTDYNAMICPARTITIONTHRESHOLD, 1);
    HiveConf.setVar(configuration, HiveConf.ConfVars.DYNAMICPARTITIONINGMODE, "nonstrict");
+    HiveConf.setFloatVar(configuration, HiveConf.ConfVars.TEZ_MAX_PARTITION_FACTOR, 1f);


Is it OK to apply this one to all vertices including non insert reducers?

I updated the solution to apply this only for write operations.

@okumin Nice catch!

okumin · 2023-07-16T06:02:38Z

+            serDeProperties.getProperty(Catalogs.NAME));
+
+    if (operation != null) {
+      HiveConf.setFloatVar(configuration, HiveConf.ConfVars.TEZ_MAX_PARTITION_FACTOR, 1f);


I tested how this works as I am not confident. Please not that I am still a beginner of Iceberg and something could be wrong with my test.

Test queries

Prep

beeline -e " DROP TABLE IF EXISTS test; CREATE TABLE test (id INT) STORED BY ICEBERG TBLPROPERTIES('format-version'='2'); INSERT INTO test VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10); "

DELETE

beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true

Test result

The original version

This is the tested revision. Two reducers are set up and launched as reported.

$ beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true ... INFO : 2023-07-16 04:55:39,740 Map 1: 0(+1)/1 Reducer 2: 0/2 INFO : 2023-07-16 04:55:42,259 Map 1: 1/1 Reducer 2: 0(+1)/2

$ ./bin/logs hive-hiveserver2-655b558bb-gzwh8 | grep HIVE-27050 | tail -n 1 hive-hiveserver2-655b558bb-gzwh8: 2023-07-16T04:55:31,760 INFO [f3ecdcb4-f45b-405f-986e-0dbf525c7c87 HiveServer2-Handler-Pool: Thread-61] parse.GenTezUtils: HIVE-27050: max partition factor=2.0, max partition=2

The first patch

This is the tested revision. One reducer is set up and launched.

$ beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true ... INFO : 2023-07-16 05:22:30,905 Map 1: 0/1 Reducer 2: 0/1 INFO : 2023-07-16 05:22:35,454 Map 1: 0(+1)/1 Reducer 2: 0/1

$ ./bin/logs hive-hiveserver2-75bc6bc94c-bhxxc | grep HIVE-27050 | tail -n 1 hive-hiveserver2-75bc6bc94c-bhxxc: 2023-07-16T05:22:23,324 INFO [18f9ace5-2016-4430-8155-ae95c254148b HiveServer2-Handler-Pool: Thread-61] parse.GenTezUtils: HIVE-27050: max partition factor=1.0, max partition=1

The second(current) path

This is the tested revision. It is unlikely to be working as expected.

$ beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true ... INFO : 2023-07-16 05:58:27,235 Map 1: 1/1 Reducer 2: 0/2 INFO : 2023-07-16 05:58:27,741 Map 1: 1/1 Reducer 2: 0(+1)/2

$ ./bin/logs hive-hiveserver2-5876b658cd-8zxwp | grep HIVE-27050 | tail -n 3 hive-hiveserver2-5876b658cd-8zxwp: 2023-07-16T05:58:13,110 INFO [62373162-813c-409b-834d-335f2f72efc1 HiveServer2-Handler-Pool: Thread-61] parse.GenTezUtils: HIVE-27050: max partition factor=2.0, max partition=2 hive-hiveserver2-5876b658cd-8zxwp: 2023-07-16T05:58:13,112 INFO [62373162-813c-409b-834d-335f2f72efc1 HiveServer2-Handler-Pool: Thread-61] hive.HiveIcebergSerDe: HIVE-27050: operation: null hive-hiveserver2-5876b658cd-8zxwp: 2023-07-16T05:58:28,411 INFO [HiveServer2-Background-Pool: Thread-111] hive.HiveIcebergSerDe: HIVE-27050: operation: null

Some random thoughts. I would say these are minor.

Is it best to disable over-provisioning only for DELETE reducers using a Hive hook or something?

Over-provisioning might work for INSERT or UPDATE

It could work also for DELETE if rows are filtered

Maybe, the optimization should be applied to only the last reducer in the case like Map -> Reduce -> Reduce?

hive.tez.auto.reducer.parallelism.min.threshold=0.0 can be an option?

Hi @okumin,
Thank you for all the info!
I tried a DELETE query with your test table with hive.tez.auto.reducer.parallelism.min.threshold=0.0 and it still gave 2 reducers.
I also noticed in debug that GenTezUtils.createReduceWork() is called before HiveIcebergSerDe.initialize() with a write operation where I tried to set max partition factor to 1 which probably explains why my change doesn't work.
What do you think, can I set max partition factor to 1.0 in GenTezUtils.createReduceWork() if the operation is a write operation for Iceberg table?

I personally think it is reasonable to explicitly inject the logic into GenTezUtils or somewhere. One request is I'd like to make it pluggable because other formats would hit the same issue. As far as I checked on my machine, Hive ACID shares the problem. Note that these are my opinions and committers could have different ideas, or they might think it is an expected behavior.

As for the parameter, I guess you tested it with 4.0.0-alpha-2 since the param was merged recently. It enforces auto reduce parallelism.

$ beeline -e " > DROP TABLE IF EXISTS test; > CREATE TABLE test (id INT) STORED BY ICEBERG TBLPROPERTIES('format-version'='2'); > INSERT INTO test VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10); > "

$ beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true ... INFO : 2023-07-18 14:19:45,200 Map 1: 0(+1)/1 Reducer 2: 0/2 INFO : 2023-07-18 14:19:48,227 Map 1: 1/1 Reducer 2: 0(+1)/2 ... $ beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true --hiveconf hive.tez.auto.reducer.parallelism.min.threshold=0.0 ... INFO : 2023-07-18 14:20:23,730 Map 1: 0(+1)/1 Reducer 2: 0/2 INFO : 2023-07-18 14:20:27,271 Map 1: 1/1 Reducer 2: 0(+1)/1

I updated the solution and placed it in GenTezUtils and made it for Iceberg and Hive ACID.

okumin · 2023-07-20T12:56:47Z

+    if (context.parseContext.getContext().getOperation() == Context.Operation.DELETE &&
+            isRestrictReducerExtrapolation(context)) {
+      LOG.debug("Overriding maxPartitionFactor to 1.0 to prevent creation of small files after delete operation");
+      maxPartitionFactor = 1f;


I quickly double-checked it works as expected. Let's wait for the comment of committers about this approach.

ACID

$ beeline -e " > DROP TABLE IF EXISTS test; > CREATE TABLE test (id INT) STORED AS ORC TBLPROPERTIES ('transactional'='true'); > INSERT INTO test VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10); > " ... $ beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true ... INFO : 2023-07-20 12:52:55,637 Map 1: -/- Reducer 2: 0/1 INFO : 2023-07-20 12:52:57,161 Map 1: 0/1 Reducer 2: 0/1

Iceberg

$ beeline -e " > DROP TABLE IF EXISTS test; > CREATE TABLE test (id INT) STORED BY ICEBERG TBLPROPERTIES('format-version'='2'); > INSERT INTO test VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10); > " ... $ beeline -e 'DELETE FROM test WHERE id = 5' --hiveconf hive.server2.in.place.progress=false --hiveconf hive.tez.auto.reducer.parallelism=true ... INFO : 2023-07-20 12:55:56,104 Map 1: -/- Reducer 2: 0/1 INFO : 2023-07-20 12:55:58,337 Map 1: 0/1 Reducer 2: 0/1

okumin · 2023-07-20T13:17:33Z

@@ -231,10 +231,17 @@ public static void validateTableIsIceberg(org.apache.hadoop.hive.ql.metadata.Tab
  }

  public static boolean isIcebergTable(Table table) {


Wow, I originally thought the change be available also for Delta Lake, Hudi, etc, but looks like we hardcode many for Iceberg...
I will create a ticket to push this kind of logic to StorageHandler...

okumin · 2023-07-20T13:19:07Z

-            table.getStorageHandler().getType() == StorageHandlerTypes.ICEBERG;
+    return table.isNonNative() &&
+            ((table.getStorageHandler() != null && table.getStorageHandler().getType() == StorageHandlerTypes.ICEBERG) || 
+                    isIcebergTableType(table.getTTable().getParameters()));


Is there any case where StorageHandler#getType != ICEBERG but isIcebergTableType = true?

no, shouldn't be

Fixed @ retested using MiniHS2Cluster for Iceberg and Hive ACID tables.

deniskuzZ · 2023-07-28T12:18:46Z

    return unionWork;
  }
+
+  private static boolean isRestrictReducerExtrapolation(GenTezProcContext context) {


could we refactor to

private static boolean isRestrictReducerExtrapolation(Context context) { return context.getOperation() == Context.Operation.DELETE && context.getLoadTableOutputMap().values() .stream().map(WriteEntity::getTable).anyMatch(DDLUtils::isIcebergTable); } -> isRestrictReducerExtrapolation(context.parseContext.getContext())

… files being created for Iceberg tables.

sonarqubecloud · 2023-07-29T03:46:35Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
No Duplication information

deniskuzZ

LGTM +1

… of small files being created for Iceberg tables (Dmitriy Fingerman, reviewed by Denys Kuzmenko, okumin) Closes apache#4477

kgyrtkirk added the tests pending label Jul 11, 2023

difin force-pushed the HIVE-27050_iceberg-reducers branch from a0db293 to 772a837 Compare July 11, 2023 17:37

aturoczy approved these changes Jul 11, 2023

View reviewed changes

kgyrtkirk added tests passed and removed tests pending labels Jul 12, 2023

okumin reviewed Jul 12, 2023

View reviewed changes

difin force-pushed the HIVE-27050_iceberg-reducers branch from 772a837 to bc4e3e1 Compare July 13, 2023 19:03

kgyrtkirk added tests pending tests passed and removed tests passed tests pending labels Jul 13, 2023

okumin reviewed Jul 16, 2023

View reviewed changes

difin force-pushed the HIVE-27050_iceberg-reducers branch from bc4e3e1 to d3432af Compare July 18, 2023 19:25

github-actions Bot requested review from abstractdog and miklosgergely July 18, 2023 19:26

kgyrtkirk added tests pending and removed tests passed labels Jul 18, 2023

difin force-pushed the HIVE-27050_iceberg-reducers branch from d3432af to 6d30d41 Compare July 18, 2023 19:39

kgyrtkirk added tests failed tests pending tests passed and removed tests pending tests failed labels Jul 18, 2023

okumin reviewed Jul 20, 2023

View reviewed changes

difin force-pushed the HIVE-27050_iceberg-reducers branch from 6d30d41 to d468336 Compare July 21, 2023 18:38

kgyrtkirk added tests pending tests unstable and removed tests passed tests pending labels Jul 21, 2023

kgyrtkirk added tests pending tests unstable and removed tests unstable tests pending labels Jul 24, 2023

difin force-pushed the HIVE-27050_iceberg-reducers branch from 2d81889 to 1ddbbdc Compare July 25, 2023 13:22

kgyrtkirk added tests pending tests passed and removed tests unstable tests pending labels Jul 25, 2023

deniskuzZ reviewed Jul 28, 2023

View reviewed changes

difin force-pushed the HIVE-27050_iceberg-reducers branch from 1ddbbdc to a2206e0 Compare July 28, 2023 16:44

kgyrtkirk added tests pending tests failed and removed tests passed tests pending labels Jul 28, 2023

difin force-pushed the HIVE-27050_iceberg-reducers branch from a2206e0 to 8ad5316 Compare July 28, 2023 22:01

kgyrtkirk added tests pending tests unstable and removed tests failed tests pending labels Jul 28, 2023

HIVE-27050: Restrict reducer extrapolation to contain number of small…

6115012

… files being created for Iceberg tables.

difin force-pushed the HIVE-27050_iceberg-reducers branch from 8ad5316 to 6115012 Compare July 29, 2023 02:35

kgyrtkirk added tests pending and removed tests unstable labels Jul 29, 2023

kgyrtkirk added tests passed and removed tests pending labels Jul 29, 2023

deniskuzZ approved these changes Jul 31, 2023

View reviewed changes

deniskuzZ merged commit 4ad7d16 into apache:master Aug 1, 2023

		@@ -231,10 +231,17 @@ public static void validateTableIsIceberg(org.apache.hadoop.hive.ql.metadata.Tab
		}

		public static boolean isIcebergTable(Table table) {

Uh oh!

Conversation

difin commented Jul 11, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

Is the change a dependency upgrade?

How was this patch tested?

Uh oh!

aturoczy left a comment

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!

okumin Jul 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Test queries

Prep

DELETE

Test result

The original version

The first patch

The second(current) path

Uh oh!

Choose a reason for hiding this comment

Uh oh!

difin Jul 17, 2023 • 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!

okumin Jul 20, 2023 • 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!

difin Jul 21, 2023 • 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!

sonarqubecloud Bot commented Jul 29, 2023

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

okumin Jul 16, 2023 •

edited

Loading

difin Jul 17, 2023 •

edited

Loading

okumin Jul 20, 2023 •

edited

Loading

difin Jul 21, 2023 •

edited

Loading