DynamoDB plugin #76
DynamoDB plugin #76
Conversation
ipapapa
requested review from
ScottMansfield,
jhspaybar,
vinaykumarchella and
shailesh33
| public String readSingle(String key) throws Exception { | ||
| Item item = null; | ||
| try { | ||
| GetItemSpec spec = new GetItemSpec().withPrimaryKey("Id", key).withConsistentRead(config.consistentRead()); |
There was a problem hiding this comment.
consistent reads will use double the RCUs and can't be served by DAX. They're also only consistent within the region and won't be globally consistent when using cross region replication.
There was a problem hiding this comment.
Right, the idea is to test it in comparison with a quorum Cassandra read. By default, we will not be using it when we want to test DAX: @DefaultValue("false").
| * @author ipapapa | ||
| */ | ||
| @Configuration(prefix = "ndbench.config.dynamodb") | ||
| public interface DynamoDBConfigs { |
There was a problem hiding this comment.
This looks interesting. I am curious about bindings though. Hopefully, this does not require explicit bindings.
There was a problem hiding this comment.
Good catch, I missed that. I need to add a NdBenchClientPluginGuiceModule to leverage the custom configuration. I will do it.
|
|
||
| private void amazonServiceException(AmazonServiceException ase) { | ||
|
|
||
| logger.error("Caught an AmazonServiceException, which means your request made it " |
There was a problem hiding this comment.
DO you want to log a single message or 6 different messages here?
There was a problem hiding this comment.
This is standard AWS way of reporting errors. In fact, each message presents some information that is useful.
There was a problem hiding this comment.
Agreed. I was just wondering if it is needed for any reason (like grepping) to have the entire message in a single line rather than split across 6 lines. I am fine either ways.
| } catch (AmazonClientException ace) { | ||
| amazonClientException(ace); | ||
| } | ||
| return item.toString(); |
There was a problem hiding this comment.
what is item here when an exception is thrown?
| } catch (AmazonClientException ace) { | ||
| amazonClientException(ace); | ||
| } | ||
| return outcome.toString(); |
| @Override | ||
| public void shutdown() throws Exception { | ||
| try { | ||
| logger.info("Issuing DeleteTable request for " + config.getTableName()); |
There was a problem hiding this comment.
why do we want to delete the table at shutdown? Can it happen that we restart the test at a pre-filled table?
There was a problem hiding this comment.
This is a good question. NDBench provides a prefill. This is the proper way to make your database hot. The issue with a managed service (table) is that you can configure with high WCUs and RCUs pay a lot of money and then forget it. After you are done with your tests we need to make sure that there is a way to clean the resources. What other options do you propose?
| */ | ||
|
|
||
| logger.debug("Creating table if it does not exist yet"); | ||
| table = dynamoDB.createTable(this.config.getTableName(), |
There was a problem hiding this comment.
what happens when the table already exists? does it fail or is ignored?
There was a problem hiding this comment.
ResourceInUseException
The operation conflicts with the resource's availability. For example, you attempted to recreate an existing table, or tried to delete a table currently in the CREATING state.
HTTP Status Code: 400
Resource: https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_CreateTable.html
| String getTableName(); | ||
|
|
||
| /* | ||
| * Attributes – Each item is composed of one or more attributes. An attribute is |
There was a problem hiding this comment.
can you elaborate on what is an item?
There was a problem hiding this comment.
| * | ||
| * @author ipapapa | ||
| */ | ||
| @Configuration(prefix = "ndbench.config.dynamodb") |
There was a problem hiding this comment.
Just to follow the standard conventions, can you change the prefix to use
NdBenchConstants.PROP_NAMESPACE +
There was a problem hiding this comment.
I saw this too late and I merged it. I will revert the changes and follow up in another PR.
This is the first cut of a simple DynamoDB plugin. There might be a few issues since I have not officially tested it yet. If you can add things that you would like to see in the comments then I can keep adding new plugins or functionalities that are important for Netflix.
In the configurations, there is the ability to configure capacity limits and read consistency guarantees. Global tables are something we want to investigate.