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

Fixing IOPs issue with lease table #360

Merged
merged 3 commits into from
Aug 13, 2018

Conversation

sahilpalvia
Copy link
Contributor

Issue #, if available:

  • DynamoDB IOPs configuration being ignored with KCL v2.0

Description of changes:

  • Making DynamoDBLeaseCoordinator take IOPs configuration in the constructor
  • InitialLeaseTableReadCapacity and InitialLeaseTableWriteCapacity for the DynamoDBLeaseCoordinator class throws UnsupportedException

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

* Making DynamoDBLeaseCoordinator take IOPs configuration in the constructor
* InitialLeaseTableReadCapacity and InitialLeaseTableWriteCapacity for the DynamoDBLeaseCoordinator class throws UnsupportedException
@sahilpalvia sahilpalvia requested a review from pfifer August 9, 2018 22:19
}
this.initialLeaseTableReadCapacity = readCapacity;
return this;
throw new UnsupportedOperationException("Please set read capacity using the constructor");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. Keep the setting available. You can mark them as deprecated though.

@@ -100,6 +110,8 @@ public DynamoDBLeaseCoordinator(final LeaseRefresher leaseRefresher,
final int maxLeasesForWorker,
final int maxLeasesToStealAtOneTime,
final int maxLeaseRenewerThreadCount,
final long initialLeaseTableReadCapacity,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we just added the @KinesisClientInternalApi retain the existing constructor and chain them together. You can mark the original constructor as deprecated.

Sahil Palvia added 2 commits August 10, 2018 10:08
* Reverting the constructor, and adding chained constructor
* Reverting support for initial iops methods
* Adding deprecated tags and notes to javadoc
* Introducing chained constructors in DynamoDBLeaseManagementFactory
* Introducing TableConstants to maintain Default IOPS in one place
@pfifer pfifer merged commit 6973152 into awslabs:master Aug 13, 2018
@pfifer pfifer added this to the v2.0.1 milestone Aug 13, 2018
@sahilpalvia sahilpalvia deleted the dynamodb-iops-fix branch August 13, 2018 16:44
@pfifer pfifer added v2.x Issues related to the 2.x version and removed v2.x Issues related to the 2.x version labels Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.x Issues related to the 2.x version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants