deepakpanda93 commented on code in PR #19591: URL: https://github.com/apache/hudi/pull/19591#discussion_r3783751495
########## website/docs/concurrency_control.md: ########## @@ -188,6 +188,32 @@ com.amazonaws:aws-java-sdk-dynamodb com.amazonaws:aws-java-sdk-core ``` +### DynamoDB-Based Lock Provider with Implicit Partition Key + +```properties +hoodie.write.lock.provider=org.apache.hudi.aws.transaction.lock.DynamoDBBasedImplicitPartitionKeyLockProvider +``` + +This variant behaves like the DynamoDB-based lock provider above, except in how it determines the DynamoDB partition +key. Rather than reading `hoodie.write.lock.dynamodb.partition_key`, it derives the key from the table's base path: the +64-bit xxHash of that path, with `s3a://` normalized to `s3://` so that writers reaching the same table through either +scheme take the same lock. + +Prefer it when many tables share one lock table. The standard provider takes its partition key from Review Comment: Thanks for re-checking against source and confirming. Agreed, no change needed here, so I am not pushing a commit for this round. For anyone reading the thread later, the wording that resolved it now reads: > Prefer it when many tables share one lock table. The standard provider takes its partition key from `hoodie.write.lock.dynamodb.partition_key`, which you rarely set: when it is absent, Hudi fills it in from the table name. The earlier draft said the standard provider "requires" the config and then that Hudi "infers it from the table name" when unset, which read as self-contradictory: it left the reader unsure whether an unset value fails or falls back. The behaviour is neither phrasing on its own. `DynamoDbBasedLockConfig.from` copies the caller props and calls `setDefaults`, and `HoodieConfig.setDefaultValue` materialises an inferred value into the props when the property has an infer function. Only afterwards does `DynamoDBBasedLockProvider.getDynamoDBPartitionKey` run `checkArgument(config.contains(DYNAMODB_LOCK_PARTITION_KEY))`, and `HoodieConfig.contains` looks only at the props map. So by the time the check runs the value is present, and "fills it in from the table name" is the accurate description. Current state of the PR: - Two commits: the original section, plus `ef92ee8ea74a` for the wording fix above. - Rebased on and containing the current `asf-site` head (`d11a5b0adee4`); GitHub reports it mergeable. - Applied to `next` and all five 1.x versioned copies. Heading level follows each page's local structure: `###` on `next`, 1.1.1 and 1.2.0, `####` on 1.0.0, 1.0.1 and 1.0.2, verified in the rendered HTML so the new heading sits at the same level as its siblings on both layouts. - `npm run build` passes with the warning set byte-identical to a baseline built from the same base commit. One thing worth being explicit about rather than leaving implied: the version scope was checked per release instead of inferred from the class merely existing, because on an earlier docs PR (#19572) a config that was present at 1.0.0 and 1.0.1 turned out to throw there. For this provider the implementation is the same on all five tags, differing only by an import moving from `S3Utils.s3aToS3` to `FSUtils.s3aToS3`, so the xxHash derivation, the `s3a` normalisation and the base-class inheritance are identical throughout. The one claim I could not verify from source alone is the note about the provider logging the base path alongside its derived key. That came from reading the provider, but I have not run it against a real DynamoDB lock table, so a committer confirming the log line actually surfaces both values would be worth having. @yihua, since you filed the original request on #16814, does the section cover what you had in mind? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
