Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HUDI-8490] Implicit lock provider use different lock key scheme #12220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Davis-Zhang-Onehouse
Copy link
Contributor

@Davis-Zhang-Onehouse Davis-Zhang-Onehouse commented Nov 7, 2024

Change Logs

code + test

Impact

For implicit lock provider org.apache.hudi.aws.transaction.lock.DynamoDBBasedImplicitPartitionKeyLockProvider and org.apache.hudi.client.transaction.lock.ZookeeperBasedImplicitBasePathLockProvider, inside the lock key (for zookeeper) / partition key (for ddb) it contains substring <tablename>-<hash of table basepath>.

This is problematic in case of alter table rename, some writers are using the old lock key while others are using the new one, which leads to concurrency bug as the lock fails to synchronize operations from concurrent operators.

Alter rename should not have coupling with locks, so to remove the dependency, we removed the table name from the lock key for both implicit ddb and zookeeper lock provider.

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

Low

Documentation Update

We should update the merge into doc about this new restriction.

Contributor's checklist

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

@github-actions github-actions bot added the size:S PR with lines of changes in (10, 100] label Nov 7, 2024
@hudi-bot
Copy link

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S PR with lines of changes in (10, 100]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants