Enable autoscaling when using programmatic tables #140

amcp · 2018-04-10T13:19:58Z

Fixes #131

I use default values present in the AWS console for all numbers (initial r/w CU, minimum r/w CU, maximum r/w CU and target r/w utilization).

ipapapa · 2018-04-11T19:47:04Z

...nch-dynamodb-plugins/src/main/java/com/netflix/ndbench/plugin/dynamodb/DynamoDBKeyValue.java

@@ -252,8 +278,8 @@ private void initializeTable() {

        logger.debug("Creating table if it does not exist yet");

-        Long readCapacityUnits = Long.parseLong(this.config.getReadCapacityUnits());
-        Long writeCapacityUnits = Long.parseLong(this.config.getWriteCapacityUnits());
+        Long readCapacityUnits = this.config.getReadCapacityUnits();


Note for myself: In my initial PR for the DDB plugin #76 I had this as Long and Eureka had some issues with it. That is why I changed it to String and did the funky parseLong stuff.

ipapapa · 2018-04-11T20:26:44Z

...nch-dynamodb-plugins/src/main/java/com/netflix/ndbench/plugin/dynamodb/DynamoDBKeyValue.java

@@ -134,6 +154,12 @@ public void init(DataGenerator dataGenerator) throws Exception {
        } else {
            dynamoDB = new DynamoDB(client);
        }
+        autoScalingClient = AWSApplicationAutoScalingClientBuilder.standard()


I assume we want to configure autoscaling if we use this.config.programmableTables() otherwise, the user should use the AWS Console to play around with Table configurations. Therefore this should fit in the initializeTable() method. Am I missing something here?

yes, programmableTables=false and autoscaling=true is an invalid configuration. ill put this logic in initialize.

ipapapa · 2018-04-11T20:29:48Z

@amcp @vinaykumarchella @jolynch @shailesh33 I am not sure if it clear in the code. There are two ways to use the plugin. One way that the user does everything through the AWS Console and the second way that the user does most Table configurations programmatically. It feels to me that the first would be the majority of the use cases. Hence this PR should fit in the initializeTable() method. An alternative approach would be to have a separate plugin one that the user configures the table programmatically and one that the user does it through the AWS console. Thoughts on which approach?

ipapapa · 2018-04-12T12:54:24Z

...nch-dynamodb-plugins/src/main/java/com/netflix/ndbench/plugin/dynamodb/DynamoDBKeyValue.java

+                writeCapacityUnits,
+                limits.getTableMaxWriteCapacityUnits().intValue(),
+                targetWriteUtilization);
+        createAndVerifyScalableTargetAndScalingPolicy(serviceRoleArn,


Is there a reason calling createAndVerifyScalableTargetAndScalingPolicy twice?

yes, once for the read utilization/capacity target and once for the write utilization/capacity target

Good point, my mistake. I missed the difference in the arguments. It might worth adding a comment above for people who read the code.

ipapapa · 2018-04-12T12:56:57Z

...nch-dynamodb-plugins/src/main/java/com/netflix/ndbench/plugin/dynamodb/DynamoDBKeyValue.java

+
+        // configure autoscaling - from aws docs
+        // https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/AutoScaling.HowTo.SDK.html
+        final String resourceID = "table/" + config.getTableName();


These final variables definition at the end of the method does not help the readability of the code. Their scope of usage is in createAndVerifyScalableTargetAndScalingPolicy hence I am wondering if it is better to define them in there. Moreover, the class is becoming too long, does it worth splitting into a different class?

removed the final and moving to a different class.

amcp · 2018-04-12T16:48:02Z

@ipapapa seems like I am going to have to do the credential refactoring first then rebase this branch on the credential refactoring. lets put this PR on hold until then.

ipapapa · 2018-04-12T16:50:46Z

OK, I will block it to avoid mistakenly merging it until you tell me to unblock it.

ipapapa

Blocking it based on author request.

ipapapa · 2018-04-15T19:37:45Z

Is there an update on this PR? is it WIP?

amcp · 2018-04-16T11:31:41Z

I've opened PR #182 to fix the story on credential injection. After merging #182, let me rebase this PR and then we can merge it.

amcp · 2018-04-16T12:18:21Z

I rebased on the cli branch for easier testing.

amcp · 2018-04-16T20:05:51Z

Setup:

sudo yum update -y \
&& sudo yum upgrade -y \
&& sudo yum install -y java-1.8.0-openjdk-devel git emacs \
&& sudo yum remove -y java-1.7.0-openjdk \
&& git clone https://github.com/amcp/ndbench.git \
&& cd ndbench \
&& git checkout consumed \
&& ./gradlew build

Test:

DISCOVERY_ENV=AWS ./gradlew run \
-Dndbench.config.cli.clientName=DynamoDBKeyValue \
-Dndbench.config.cli.bulkSize=1 \
-Dndbench.config.cli.timeoutMillis=600000 \
-Dndbench.config.dataSize=1000 \
-Dndbench.config.numKeys=1 \
-Dndbench.config.readEnabled=true \
-Dndbench.config.readRateLimit=4 \
-Dndbench.config.writeEnabled=true \
-Dndbench.config.writeRateLimit=4 \
-Dndbench.config.dynamodb.rcu=5 \
-Dndbench.config.dynamodb.wcu=5 \
-Dndbench.config.dynamodb.tablename=auto \
-Dndbench.config.dynamodb.consistentread=true \
-Dndbench.config.dynamodb.maxRetries=0 \
-Dndbench.config.dynamodb.requestTimeout=100 \
-Dndbench.config.dynamodb.maxConnections=1 \
-Dndbench.config.dynamodb.numReaders=1 \
-Dndbench.config.dynamodb.numWriters=1 \
-Dndbench.config.dynamodb.programmableTables=true

Partially addresses Netflix#118

Fixes Netflix#131

amcp force-pushed the autoscale branch from 6ef02b3 to 31ace4e Compare April 10, 2018 13:23

vinaykumarchella requested a review from ipapapa April 10, 2018 21:13

amcp force-pushed the autoscale branch 3 times, most recently from c815697 to 2a59e8c Compare April 11, 2018 18:58

ipapapa reviewed Apr 11, 2018

View reviewed changes

amcp force-pushed the autoscale branch from 2a59e8c to b17767c Compare April 11, 2018 20:15

ipapapa reviewed Apr 11, 2018

View reviewed changes

ipapapa added the enhancement label Apr 11, 2018

amcp force-pushed the autoscale branch from b17767c to bbd0657 Compare April 12, 2018 05:12

ipapapa reviewed Apr 12, 2018

View reviewed changes

ipapapa mentioned this pull request Apr 12, 2018

Add max retries and max request timeout to config #149

Merged

amcp force-pushed the autoscale branch from bbd0657 to 260915e Compare April 12, 2018 16:46

amcp force-pushed the autoscale branch from 260915e to b89c65f Compare April 12, 2018 16:50

ipapapa suggested changes Apr 12, 2018

View reviewed changes

amcp force-pushed the autoscale branch 2 times, most recently from 761f1da to 442a17c Compare April 14, 2018 06:32

amcp force-pushed the autoscale branch 4 times, most recently from 53fcc9c to dde7e12 Compare April 16, 2018 11:29

amcp force-pushed the autoscale branch from dde7e12 to b2aacca Compare April 16, 2018 12:01

amcp force-pushed the autoscale branch 2 times, most recently from c5e535b to d5e15a1 Compare April 16, 2018 20:21

amcp closed this Apr 16, 2018

amcp reopened this Apr 16, 2018

amcp force-pushed the autoscale branch 2 times, most recently from 10b332b to f0f1455 Compare April 18, 2018 18:42

Add cli driver

658f66b

Partially addresses Netflix#118

amcp force-pushed the autoscale branch from f0f1455 to 14a8309 Compare April 18, 2018 21:11

Enable autoscaling when using programmatic tables

b3df134

Fixes Netflix#131

amcp force-pushed the autoscale branch from 14a8309 to b3df134 Compare April 18, 2018 21:12

ipapapa merged commit 0bd861f into Netflix:master Apr 19, 2018

amcp deleted the autoscale branch April 19, 2018 20:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable autoscaling when using programmatic tables #140

Enable autoscaling when using programmatic tables #140

amcp commented Apr 10, 2018

ipapapa Apr 11, 2018

amcp Apr 11, 2018

ipapapa Apr 11, 2018 •

edited

amcp Apr 12, 2018

ipapapa commented Apr 11, 2018 •

edited

ipapapa Apr 12, 2018 •

edited

amcp Apr 12, 2018

ipapapa Apr 12, 2018 •

edited

ipapapa Apr 12, 2018

amcp Apr 12, 2018

amcp commented Apr 12, 2018

ipapapa commented Apr 12, 2018 •

edited

ipapapa left a comment

ipapapa commented Apr 15, 2018 •

edited

amcp commented Apr 16, 2018

amcp commented Apr 16, 2018

amcp commented Apr 16, 2018 •

edited

Enable autoscaling when using programmatic tables #140

Enable autoscaling when using programmatic tables #140

Conversation

amcp commented Apr 10, 2018

ipapapa Apr 11, 2018

Choose a reason for hiding this comment

amcp Apr 11, 2018

Choose a reason for hiding this comment

ipapapa Apr 11, 2018 • edited

Choose a reason for hiding this comment

amcp Apr 12, 2018

Choose a reason for hiding this comment

ipapapa commented Apr 11, 2018 • edited

ipapapa Apr 12, 2018 • edited

Choose a reason for hiding this comment

amcp Apr 12, 2018

Choose a reason for hiding this comment

ipapapa Apr 12, 2018 • edited

Choose a reason for hiding this comment

ipapapa Apr 12, 2018

Choose a reason for hiding this comment

amcp Apr 12, 2018

Choose a reason for hiding this comment

amcp commented Apr 12, 2018

ipapapa commented Apr 12, 2018 • edited

ipapapa left a comment

Choose a reason for hiding this comment

ipapapa commented Apr 15, 2018 • edited

amcp commented Apr 16, 2018

amcp commented Apr 16, 2018

amcp commented Apr 16, 2018 • edited

ipapapa Apr 11, 2018 •

edited

ipapapa commented Apr 11, 2018 •

edited

ipapapa Apr 12, 2018 •

edited

ipapapa Apr 12, 2018 •

edited

ipapapa commented Apr 12, 2018 •

edited

ipapapa commented Apr 15, 2018 •

edited

amcp commented Apr 16, 2018 •

edited