[HUDI-5575] Adding/Fixing auto generation of record keys w/ hudi #7726

nsivabalan · 2023-01-21T23:00:05Z

Change Logs

As of now, record key generation and partition path generation are tightly coupled based on the key gen class used. Recently we added auto generation of record keys, its not flexible to be used w/ any key gen class as its a separate key gen class in itself.

This patch fixes that so that users can enable auto generation of record keys along w/ any key gen class. Users have to enable a config called hoodie.auto.generate.record.keys to let hudi auto generate record keys for them. In such cases, users don't need to set any value for hoodie.datasource.write.recordkey.field which otherwise is a mandatory field. When this new config is enabled, the key gen class config (hoodie.datasource.write.keygenerator.class) will determine the partition path generation logic as before.

Added a new class called AutoRecordKeyGenerator which will be leveraged in all key gen classes for record key related methods.

Impact

Enables users enable auto generation of record keys along w/ any key gen class.

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

low.

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

hudi-common/src/main/java/org/apache/hudi/keygen/BaseKeyGenerator.java

...ient/hudi-client-common/src/test/java/org/apache/hudi/keygen/TestAutoRecordKeyGenerator.java

.../hudi-client-common/src/main/java/org/apache/hudi/keygen/TimestampBasedAvroKeyGenerator.java

hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/ComplexKeyGenerator.java

hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/CustomKeyGenerator.java

...spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/TestAutoRecordKeyGeneration.scala

...-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/ComplexAvroKeyGenerator.java

nsivabalan · 2023-01-23T22:31:45Z

@codope : addressed all feedback.

codope

Thanks for addressing the comments. Ready to land once the CI is green.

…ey gen classes

hudi-bot · 2023-01-24T12:09:20Z

CI report:

efe71f4 Azure: FAILURE
7de21a1 Azure: PENDING

Bot commands

@hudi-bot supports the following commands:

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

codope · 2023-01-24T14:34:28Z

Merging it as all tests are fixed and only testUpsertsContinuousModeWithMultipleWritersForConflicts is flaky, which is expected to be fixed by #7720

alexeykudinkin · 2023-01-24T18:52:11Z

.../hudi-client-common/src/main/java/org/apache/hudi/keygen/NonpartitionedAvroKeyGenerator.java

    this.recordKeyFields = Arrays.stream(props.getString(KeyGeneratorOptions.RECORDKEY_FIELD_NAME.key())
        .split(",")).map(String::trim).filter(s -> !s.isEmpty()).collect(Collectors.toList());
    this.partitionPathFields = EMPTY_PARTITION_FIELD_LIST;
+    instantiateAutoRecordKeyGenerator();


Please assign var inside the ctor, make it final

alexeykudinkin · 2023-01-24T18:53:13Z

hudi-common/src/main/java/org/apache/hudi/keygen/BaseKeyGenerator.java

  protected final boolean hiveStylePartitioning;
  protected final boolean consistentLogicalTimestampEnabled;
+  protected final boolean autoGenerateRecordKeys;
+  protected AutoRecordKeyGenerator autoRecordKeyGenerator;


Please wrap it into Option

alexeykudinkin · 2023-01-24T18:53:55Z

hudi-common/src/main/java/org/apache/hudi/keygen/BaseKeyGenerator.java

  protected final boolean encodePartitionPath;
  protected final boolean hiveStylePartitioning;
  protected final boolean consistentLogicalTimestampEnabled;
+  protected final boolean autoGenerateRecordKeys;


We don't need a separate boolean, let's hold auto-gen as an option

alexeykudinkin · 2023-01-24T18:56:09Z

hudi-common/src/main/java/org/apache/hudi/keygen/constant/KeyGeneratorOptions.java

+          + "The class attempts to get sufficient uniqueness for keys to prevent frequent collisions by choosing the fields it uses in order of decreasing "
+          + "likelihood for uniqueness.");
+
+  public static final ConfigProperty<Integer> NUM_FIELDS_IN_AUTO_RECORDKEY_GENERATION = ConfigProperty


I don't think we should expose this kind of configuration -- it's very confusing in terms of what exactly it affects, and how setting it will translate into desired outcome

alexeykudinkin · 2023-01-24T19:00:41Z

...ient/hudi-client-common/src/test/java/org/apache/hudi/keygen/TestAutoRecordKeyGenerator.java

  public void createKeyWithoutPartitionColumn() {
-    KeylessKeyGenerator keyGenerator = new KeylessKeyGenerator(getKeyGenProperties("", 3));
-    GenericRecord record = createRecord("partition1", "value1", 123, 456L, TIME, null);
+    ComplexAvroKeyGenerator keyGenerator = new ComplexAvroKeyGenerator(getKeyGenProperties("partition_field", 3));


Why are we testing ComplexKeyGenerator here?

We need to test 2 concerns:

AutoRecordKeyGenerator itself

All existing key-generators in auto-gen mode (just one parameterized test for getRecordKey should be fine)

alexeykudinkin · 2023-01-24T19:04:10Z

hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/ComplexKeyGenerator.java

-    tryInitRowAccessor(schema);
-    return combineCompositeRecordKeyUnsafe(rowAccessor.getRecordKeyParts(internalRow));
+    if (autoGenerateRecordKeys) {
+      return super.getRecordKey(internalRow, schema);


Why are we redirecting to BuiltinKeyGenerator here?
On top of that i don't see that we've changed it either, so how is this supposed to work?

vinothchandar · 2023-01-25T04:42:27Z

@nsivabalan are @alexeykudinkin 's review feedback addressed?

…udi (apache#7726)" This reverts commit 2fc20c1.

…udi (#7726)" (#7747) * Revert "[HUDI-5575] Adding/Fixing auto generation of record keys w/ hudi (#7726)" This reverts commit 2fc20c1. * Revert "[HUDI-5514] Add in support for a keyless workflow by building an ID based off of values within the record (#7640)" This reverts commit eacae1e.

…che#7726) * Adding auto generation of record keys w/ hudi. Added support to all key gen classes * addressing feedback * Fix tests Co-authored-by: Sagar Sumit <[email protected]>

…udi (apache#7726)" (apache#7747) * Revert "[HUDI-5575] Adding/Fixing auto generation of record keys w/ hudi (apache#7726)" This reverts commit 2fc20c1. * Revert "[HUDI-5514] Add in support for a keyless workflow by building an ID based off of values within the record (apache#7640)" This reverts commit eacae1e.

…che#7726) * Adding auto generation of record keys w/ hudi. Added support to all key gen classes * addressing feedback * Fix tests Co-authored-by: Sagar Sumit <[email protected]>

…udi (apache#7726)" (apache#7747) * Revert "[HUDI-5575] Adding/Fixing auto generation of record keys w/ hudi (apache#7726)" This reverts commit 2fc20c1. * Revert "[HUDI-5514] Add in support for a keyless workflow by building an ID based off of values within the record (apache#7640)" This reverts commit eacae1e.

nsivabalan added priority:blocker Production down; release blocker writer-core labels Jan 21, 2023

nsivabalan changed the title ~~[HUDI-5575] Adding auto generation of record keys w/ hudi. Added support to all key gen clases~~ [HUDI-5575] Adding/Fixing auto generation of record keys w/ hudi Jan 21, 2023

codope reviewed Jan 23, 2023

View reviewed changes

nsivabalan force-pushed the autoGenerationRecKeys branch from 6561f2b to efe71f4 Compare January 23, 2023 22:31

codope approved these changes Jan 24, 2023

View reviewed changes

nsivabalan and others added 3 commits January 24, 2023 16:34

Adding auto generation of record keys w/ hudi. Added support to all k…

707c977

…ey gen classes

addressing feedback

3f3e87b

Fix tests

7de21a1

codope force-pushed the autoGenerationRecKeys branch from efe71f4 to 7de21a1 Compare January 24, 2023 11:04

codope merged commit 2fc20c1 into apache:master Jan 24, 2023

alexeykudinkin reviewed Jan 24, 2023

View reviewed changes

codope added a commit to codope/hudi that referenced this pull request Jan 25, 2023

Revert "[HUDI-5575] Adding/Fixing auto generation of record keys w/ h…

880eacf

…udi (apache#7726)" This reverts commit 2fc20c1.

hudi-bot mentioned this pull request Dec 9, 2025

Support any record key generation along w/ any partition path generation for row writer #15699

Open

[HUDI-5575] Adding/Fixing auto generation of record keys w/ hudi #7726

[HUDI-5575] Adding/Fixing auto generation of record keys w/ hudi #7726

Uh oh!

Conversation

nsivabalan commented Jan 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Logs

Impact

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

Documentation Update

Contributor's checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nsivabalan commented Jan 23, 2023

Uh oh!

codope left a comment

Choose a reason for hiding this comment

Uh oh!

hudi-bot commented Jan 24, 2023

CI report:

Uh oh!

codope commented Jan 24, 2023

Uh oh!

alexeykudinkin Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

alexeykudinkin Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

alexeykudinkin Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

alexeykudinkin Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

alexeykudinkin Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

alexeykudinkin Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

vinothchandar commented Jan 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nsivabalan commented Jan 21, 2023 •

edited

Loading