You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "yihua (via GitHub)" <gi...@apache.org> on 2023/06/09 20:47:50 UTC
[GitHub] [hudi] yihua commented on a diff in pull request #8868: [HUDI-6278] Fixed the use of DynamoDBLockConfig class
yihua commented on code in PR #8868:
URL: https://github.com/apache/hudi/pull/8868#discussion_r1224755814
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -185,14 +183,14 @@ private void createLockTableInDynamoDB(AmazonDynamoDB dynamoDB, String tableName
createTableRequest.setBillingMode(billingMode);
if (billingMode.equals(BillingMode.PROVISIONED.name())) {
createTableRequest.setProvisionedThroughput(new ProvisionedThroughput()
- .withReadCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY.key())))
- .withWriteCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY.key()))));
+ .withReadCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY)))
+ .withWriteCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY))));
}
dynamoDB.createTable(createTableRequest);
LOG.info("Creating dynamoDB table " + tableName + ", waiting for table to be active");
try {
- TableUtils.waitUntilActive(dynamoDB, tableName, Integer.parseInt(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_CREATION_TIMEOUT.key())), 20 * 1000);
+ TableUtils.waitUntilActive(dynamoDB, tableName, Integer.parseInt(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_CREATION_TIMEOUT)), 20 * 1000);
Review Comment:
nit: use `dynamoDBLockConfiguration.getInt`?
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -71,19 +69,19 @@ public class DynamoDBBasedLockProvider implements LockProvider<LockItem> {
private final AmazonDynamoDBLockClient client;
private final String tableName;
private final String dynamoDBPartitionKey;
- protected LockConfiguration lockConfiguration;
+ protected DynamoDbBasedLockConfig dynamoDBLockConfiguration;
Review Comment:
nit: this should be final.
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -201,14 +199,10 @@ private void createLockTableInDynamoDB(AmazonDynamoDB dynamoDB, String tableName
LOG.info("Created dynamoDB table " + tableName);
}
- private void checkRequiredProps(final LockConfiguration config) {
- ValidationUtils.checkArgument(config.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_NAME.key()) != null);
- ValidationUtils.checkArgument(config.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_REGION.key()) != null);
- ValidationUtils.checkArgument(config.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_PARTITION_KEY.key()) != null);
- config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_BILLING_MODE.key(), BillingMode.PAY_PER_REQUEST.name());
- config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY.key(), "20");
- config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY.key(), "10");
- config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_CREATION_TIMEOUT.key(), "600000");
+ private void checkRequiredProps(final DynamoDbBasedLockConfig config) {
+ ValidationUtils.checkArgument(config.contains(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_NAME.key()));
+ ValidationUtils.checkArgument(config.contains(DynamoDbBasedLockConfig.DYNAMODB_LOCK_REGION.key()));
+ ValidationUtils.checkArgument(config.contains(DynamoDbBasedLockConfig.DYNAMODB_LOCK_PARTITION_KEY.key()));
Review Comment:
nit: move these to the builder of `DynamoDbBasedLockConfig`?
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -185,14 +183,14 @@ private void createLockTableInDynamoDB(AmazonDynamoDB dynamoDB, String tableName
createTableRequest.setBillingMode(billingMode);
if (billingMode.equals(BillingMode.PROVISIONED.name())) {
createTableRequest.setProvisionedThroughput(new ProvisionedThroughput()
- .withReadCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY.key())))
- .withWriteCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY.key()))));
+ .withReadCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY)))
+ .withWriteCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY))));
Review Comment:
nit: use `dynamoDBLockConfiguration.getLong`?
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org