AWS SDK v1 DynamoDB backend #142

akhomchenko · 2020-12-30T08:04:13Z

Good evening.

Thank you a lot for awesome library. It looks really cool.

I wanted to use it for one of my projects and noticed lack of most common backend I use - AWS DynamoDB. At the time I've forked the code there was no 6.0 branch and so I based my code on 5.0 branch which had a lot of helpful abstract classes. Implementation was really simple.

Given that there is 6.0 that does not contain changes from 5.0 I have couple of questions:

is 5.0 abandoned and will not be merged to master?
if so - what is a best way to start with custom backend in master/6.0?
if not - do you plan to release it soon?

General questions:

are you interested in this contribution? It is WIP now but I will add more tests and docs to make it a little bit easier to use
any special requirements? Current implementation passes tck test but maybe something you want me to double check
do you have import, naming or formatting rules? Repository does not contain them. I am trying to follow your style (feel free to call out).

Notes:

I can take a look on SDKv2 later as I haven't used it at all (unlike v1). I assume there are common things that can be extracted.

Thank you and happy holidays!

vladimir-bukhtoyarov · 2020-12-30T08:47:05Z

Hello @akhomchenko

Do not worry. Efforts from 5.0 are not abandoned. 5.0 has been just renamed to 7.0. 7.0 by itself will be released after completion of documentation and javadocs.

It was just a technical decision to use 6.0 branch instead of 4.11, because of backward-incompatible changes in the API(replaceConfiguration now accepts the second parameter) requires from us to increase the major version, but 5.0 version was already taken by Beta release without documentation. So I decided to never release 5.0 officially and just move all the efforts to the 7.0 and release it officially in 2021.

vladimir-bukhtoyarov · 2020-12-30T08:50:11Z

According to this request: I will merge it to ```7.0```` during the holidays. Thank you for your work.

vladimir-bukhtoyarov · 2021-01-15T10:13:32Z

@ akhomchenko hello,

I am going to review and merge this request. Did you finalize all planned works?

akhomchenko · 2021-01-15T19:00:10Z

Hey, sorry for the delay.

Did you finalize all planned works?

I have update with documentation + extra safe guards locally. I will publish it by EOD for review. There are some things that I am not sure about and I want to hear your opinion as you are a lead maintainer.

I want to test it in personal AWS account during this weekend but that should not affect the review.

vladimir-bukhtoyarov · 2021-01-15T22:17:40Z

Ok. Just do not forget to remove WIP prefix when finishing.

akhomchenko · 2021-01-16T09:46:21Z

Updated. I left some comments and questions as TODOs and will add some as inline comments.

I used CFN template I specified in javadoc to create a table and used that table in StringDynamoDBBackendTckTest. All tests pass.

akhomchenko · 2021-01-16T09:48:06Z

experimental/bucket4j-dynamodb-sdk-v1/pom.xml

+            <dependency>
+                <groupId>com.amazonaws</groupId>
+                <artifactId>aws-java-sdk-bom</artifactId>
+                <version>1.11.913</version>


Unfortunately I can not specify range for dependencyManagement so I am not sure what is the best path forward. My idea was to specify 1.11.913 <2.0 range to support updates. I can update pom.xml to not use dependencyManagement and just specify range on a dependency.

akhomchenko · 2021-01-16T09:58:31Z

...4j-dynamodb-sdk-v1/src/main/java/io/github/bucket4j/dynamodb/v1/BaseDynamoDBTransaction.java

+        }
+
+        AttributeValue state = result.get(stateAttr);
+        if (state.getB() == null) {


This is the first place I have a question for. So in my initial implementation I was returning empty optional here as I was not sure if throwing an exception from get is something that can be expected (method does not define if exception can be thrown if state is broken).

I switched to throwing as there is no possibility for code to succeed if state attribute has type different from NULL or B.

This method does not support NULL value for state as from current logic NULL should never be written, but I can update code to support it.

By "current logic NULL should never be written" I mean I assume that getModifiedStateBytes of GenericEntry will not return null. Is it a case?

akhomchenko · 2021-01-16T10:03:24Z

...-dynamodb-sdk-v1/src/main/java/io/github/bucket4j/dynamodb/v1/StringDynamoDBTransaction.java

+        int length = key.getBytes(StandardCharsets.UTF_8).length;
+        if (length > MAX_SUPPORTED_LENGTH) {
+            throw new IllegalArgumentException(
+                    "key " + key + " has length of " + length + " bytes " +
+                    "while max allowed is " + MAX_SUPPORTED_LENGTH + " bytes"
+            );
+        }


I added this check in updated revision to provide better message. If you think that it is not worth the trouble and / or expensive to do I can drop it. DynamoDB will throw it's own exception.

Backend supports 2 key types: string and number (only long for now). Backend is based on compare and swap and relies on DynamoDB's condition expression to guarantee consistency.

akhomchenko · 2021-01-21T08:51:04Z

Oh wow, that was fast! Thank you for accepting my contribution! Could you please take a look on questions I left in this PR? I will do some follow-up fixed based on your feedback if necessary. I am mostly interested if GenericEntry#getModifiedStateBytes can ever return null. Thank you.

vladimir-bukhtoyarov · 2021-01-28T17:37:13Z

I will answer your questions at weekend

vladimir-bukhtoyarov changed the base branch from 5.0 to 7.0 December 30, 2020 08:58

akhomchenko force-pushed the feature/dynamodb-support branch 3 times, most recently from 8ef57c1 to e3065e6 Compare January 3, 2021 22:08

akhomchenko force-pushed the feature/dynamodb-support branch from e3065e6 to 6418c71 Compare January 16, 2021 09:45

akhomchenko changed the title ~~WIP AWS SDK v1 DynamoDB backend~~ AWS SDK v1 DynamoDB backend Jan 16, 2021

akhomchenko commented Jan 16, 2021

View reviewed changes

akhomchenko force-pushed the feature/dynamodb-support branch from 6418c71 to f47bee8 Compare January 16, 2021 10:07

add AWS DynamoDB backend via AWS SDK v1

1f8b3af

Backend supports 2 key types: string and number (only long for now). Backend is based on compare and swap and relies on DynamoDB's condition expression to guarantee consistency.

akhomchenko force-pushed the feature/dynamodb-support branch from f47bee8 to 1f8b3af Compare January 16, 2021 10:14

vladimir-bukhtoyarov merged commit 4c8d4ab into bucket4j:7.0 Jan 19, 2021

vladimir-bukhtoyarov added a commit that referenced this pull request Jan 19, 2021

#142 Moving dynamodb support to root

dce3ebd

akhomchenko deleted the feature/dynamodb-support branch January 26, 2021 00:11

vladimir-bukhtoyarov mentioned this pull request Feb 17, 2021

Apache Ignite thin client support #153

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AWS SDK v1 DynamoDB backend #142

AWS SDK v1 DynamoDB backend #142

akhomchenko commented Dec 30, 2020 •

edited

vladimir-bukhtoyarov commented Dec 30, 2020 •

edited

vladimir-bukhtoyarov commented Dec 30, 2020

vladimir-bukhtoyarov commented Jan 15, 2021

akhomchenko commented Jan 15, 2021

vladimir-bukhtoyarov commented Jan 15, 2021

akhomchenko commented Jan 16, 2021 •

edited

akhomchenko Jan 16, 2021 •

edited

akhomchenko Jan 16, 2021

akhomchenko Jan 16, 2021 •

edited

akhomchenko Jan 16, 2021

akhomchenko commented Jan 21, 2021

vladimir-bukhtoyarov commented Jan 28, 2021

AWS SDK v1 DynamoDB backend #142

AWS SDK v1 DynamoDB backend #142

Conversation

akhomchenko commented Dec 30, 2020 • edited

vladimir-bukhtoyarov commented Dec 30, 2020 • edited

vladimir-bukhtoyarov commented Dec 30, 2020

vladimir-bukhtoyarov commented Jan 15, 2021

akhomchenko commented Jan 15, 2021

vladimir-bukhtoyarov commented Jan 15, 2021

akhomchenko commented Jan 16, 2021 • edited

akhomchenko Jan 16, 2021 • edited

Choose a reason for hiding this comment

akhomchenko Jan 16, 2021

Choose a reason for hiding this comment

akhomchenko Jan 16, 2021 • edited

Choose a reason for hiding this comment

akhomchenko Jan 16, 2021

Choose a reason for hiding this comment

akhomchenko commented Jan 21, 2021

vladimir-bukhtoyarov commented Jan 28, 2021

akhomchenko commented Dec 30, 2020 •

edited

vladimir-bukhtoyarov commented Dec 30, 2020 •

edited

akhomchenko commented Jan 16, 2021 •

edited

akhomchenko Jan 16, 2021 •

edited

akhomchenko Jan 16, 2021 •

edited