[HUDI-5739] Review Hudi Config Defaults #7912

jonvex · 2023-02-09T19:00:07Z

Change Logs

Describe context and summary for this change. Highlight if any code was copied.

Impact

Describe any public API or user-facing feature change or any performance impact.

Risk level (write none, low medium or high below)

If medium or high, explain what verification was done to mitigate the risks.

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change

The config description must be updated if new configs are added or the default value of the configs are changed
Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the instruction to make
changes to the website.

Contributor's checklist

Read through contributor's guide
Change Logs and Impact were stated clearly
Adequate tests were added if applicable
CI passed

vinothchandar

Pass 1. Till HoodieWriteConfig.

vinothchandar · 2023-03-02T00:04:32Z

hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java

  public static final String DYNAMODB_BASED_LOCK_PROPERTY_PREFIX = LOCK_PREFIX + "dynamodb.";

+  // Ethan: Can we provide a default value here that is less likely to collide,
+  // So this config also becomes optional?


the lock provider should do use sth like hudi_locks as the default table name? then the user can override if needed.

Tracked in HUDI-5782

vinothchandar · 2023-03-02T00:07:17Z

hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java

      .withDocumentation("For DynamoDB based lock provider, the region used in endpoint for Amazon DynamoDB service."
                         + " Would try to first get it from AWS_REGION environment variable. If not find, by default use us-east-1");

+  // Ethan: Need to confirm these are good defaults for cost


I think this is fine.

vinothchandar · 2023-03-02T00:07:49Z

hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java

      .sinceVersion("0.10.0")
      .withDocumentation("For DynamoDB based lock provider, write capacity units when using PROVISIONED billing mode");

+  // Ethan: 10 min.  Is this timeout too long?


shorten to 2?

Tracked in HUDI-5782

vinothchandar · 2023-03-02T00:09:06Z

hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java

      .sinceVersion("0.10.0")
      .withDocumentation("For DynamoDB based lock provider, the maximum number of milliseconds to wait for creating DynamoDB table");

+  // Ethan: Move the infer logic here


+1 for sane default

Tracked in HUDI-5892

vinothchandar · 2023-03-02T00:10:35Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieArchivalConfig.java

 public class HoodieArchivalConfig extends HoodieConfig {
+  //cfg

+  // Ethan: Should we remove this?  Archive should always be enabled?


should this become an unsafe/advanced config instead?

Tracked in HUDI-5893

vinothchandar · 2023-03-02T01:20:56Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

      .withDocumentation("When enabled, data validation checks are performed during merges to ensure expected "
          + "number of records after merge operation.");

+  // Ethan: should "hoodie.combine.before.insert" controls the behavior this one intends to control as well?


logically makes sense. but I would check the code for side effects.

I'll go through the code. @nsivabalan do you have any concern around this?

Tracked in HUDI-5892 for now

vinothchandar · 2023-03-02T01:22:27Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

      .withDocumentation("When enabled, we allow duplicate keys even if inserts are routed to merge with an existing file (for ensuring file sizing)."
          + " This is only relevant for insert operation, since upsert, delete operations will ensure unique key constraints are maintained.");

+  // Ethan: should we make the default 0, to reduce the write amplication and make MOR write fast?


leave this alone.

vinothchandar · 2023-03-02T01:23:28Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

   * Given the importance of supporting such cases for the user's migration to 0.5.x, we are proposing a safety flag
   * (disabled by default) which will allow this old behavior.
   */
+  // Ethan: is this still needed?


this is used by uber. we should confirm with them.

vinothchandar · 2023-03-02T01:24:06Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

      .withDocumentation("Whether to allow generation of empty commits, even if no data was written in the commit. "
          + "It's useful in cases where extra metadata needs to be published regardless e.g tracking source offsets when ingesting data");

+  // Ethan: maybe this can auto turned on for CDC?


idk if we add this for CDC. do we?

vinothchandar · 2023-03-02T01:24:38Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

      .sinceVersion("0.11.0")
      .withDocumentation("Control to enable release all persist rdds when the spark job finish.");

+  // Ethan: shall we remove this and do auto adjustment everywhere?


we should to the extent that its reliable.

@nsivabalan let's discuss this too

Tracked in HUDI-5782

hudi-bot · 2023-03-02T04:32:37Z

CI report:

4a1bae3 UNKNOWN
ca8557f Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

yihua

I created a few JIRA tasks to scope the work to be done and parallelized.

yihua · 2023-03-07T23:37:03Z

hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java

  public static final String DYNAMODB_BASED_LOCK_PROPERTY_PREFIX = LOCK_PREFIX + "dynamodb.";

+  // Ethan: Can we provide a default value here that is less likely to collide,
+  // So this config also becomes optional?


Tracked in HUDI-5782

yihua · 2023-03-07T23:37:49Z

hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java

      .sinceVersion("0.10.0")
      .withDocumentation("For DynamoDB based lock provider, write capacity units when using PROVISIONED billing mode");

+  // Ethan: 10 min.  Is this timeout too long?


Tracked in HUDI-5782

yihua · 2023-03-07T23:40:11Z

hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java

      .sinceVersion("0.10.0")
      .withDocumentation("For DynamoDB based lock provider, the maximum number of milliseconds to wait for creating DynamoDB table");

+  // Ethan: Move the infer logic here


Tracked in HUDI-5892

yihua · 2023-03-07T23:41:31Z

hudi-aws/src/main/java/org/apache/hudi/config/HoodieAWSConfig.java

 public class HoodieAWSConfig extends HoodieConfig {
+  // AWS Access Configs
+
+  // Ethan: These configs should only be necessary if no environmental variables are not set (to confirm with code)


Tracked in HUDI-5892

yihua · 2023-03-07T23:44:47Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieArchivalConfig.java

 public class HoodieArchivalConfig extends HoodieConfig {
+  //cfg

+  // Ethan: Should we remove this?  Archive should always be enabled?


Tracked in HUDI-5893

yihua · 2023-03-08T01:54:29Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

      .sinceVersion("0.11.0")
      .withDocumentation("Control to enable release all persist rdds when the spark job finish.");

+  // Ethan: shall we remove this and do auto adjustment everywhere?


@nsivabalan let's discuss this too

yihua · 2023-03-08T01:54:57Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

      .sinceVersion("0.11.0")
      .withDocumentation("Control to enable release all persist rdds when the spark job finish.");

+  // Ethan: shall we remove this and do auto adjustment everywhere?


Tracked in HUDI-5782

yihua · 2023-03-08T01:55:13Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

      .sinceVersion("0.11.0")
      .withDocumentation("Auto adjust lock configurations when metadata table is enabled and for async table services.");

+  // Ethan: should we remove this and make the validation mandatory?


Tracked in HUDI-5782

yihua · 2023-03-08T02:08:37Z

hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java

        + "This table maintains the metadata about a given Hudi table (e.g file listings) "
        + " to avoid overhead of accessing cloud storage, during queries.")
 public final class HoodieMetadataConfig extends HoodieConfig {
+  //cfg


Tacked in HUDI-5900

yihua · 2023-03-08T02:10:01Z

hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java

      .noDefaultValue()
      .withDocumentation("Version of timeline used, by the table.");

+  // Ethan: to be merged with write payload class


Tracked in HUDI-5892

yihua · 2023-03-09T20:32:05Z

hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java

      .defaultValue("gzip")
      .withDocumentation("Compression Codec for parquet files");

+  // Ethan: Need to annotate the supported Hudi versions


"hoodie.parquet.compression.codec" default to "snappy"?

yihua · 2023-04-14T22:34:45Z

Closing this as we're done with the review of important configs.

yihua self-assigned this Feb 9, 2023

yihua changed the title ~~marked config classes~~ Review Hudi Config Defaults Feb 10, 2023

yihua changed the title ~~Review Hudi Config Defaults~~ [HUDI-5739] Review Hudi Config Defaults Feb 14, 2023

vinothchandar assigned vinothchandar and bvaradar Feb 16, 2023

Jonathan Vexler and others added 4 commits March 1, 2023 10:46

marked config classes

337ab69

found some more

85d0754

Review configs - batch 1

2b6ac18

Review configs - batch 2

a5e96b5

yihua force-pushed the mark_all_configs branch from a7c9a35 to a5e96b5 Compare March 1, 2023 18:47

yihua added 7 commits March 1, 2023 11:26

Review configs - batch 3

5a345a0

Review configs - batch 4

d0be686

Review configs - batch 5

debd0a9

Review configs - batch 6

4a1bae3

Review configs - batch 7

0dbcb68

Review configs - batch 8

e047329

Review configs - batch 9

ca8557f

vinothchandar reviewed Mar 2, 2023

View reviewed changes

yihua reviewed Mar 8, 2023

View reviewed changes

yihua reviewed Mar 9, 2023

View reviewed changes

xiaochen-zhou mentioned this pull request Mar 26, 2023

[HUDI-5893] Mark advanced configs #8295

Merged

4 tasks

yihua closed this Apr 14, 2023

This was referenced Nov 30, 2025

Code refactoring for config defaults and wiring #15829

Open

Simplify write configs for bootstrap #15830

Open

Improve config description #15831

Open

Improve config structure for table services #15832

Open

[HUDI-5739] Review Hudi Config Defaults #7912

[HUDI-5739] Review Hudi Config Defaults #7912

Uh oh!

Conversation

jonvex commented Feb 9, 2023

Change Logs

Impact

Risk level (write none, low medium or high below)

Documentation Update

Contributor's checklist

Uh oh!

vinothchandar 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!

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!

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!

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!

hudi-bot commented Mar 2, 2023

CI report:

Uh oh!

yihua 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!

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!