Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

Feature/dynamo store #221

Open
wants to merge 31 commits into
base: dev
Choose a base branch
from
Open

Feature/dynamo store #221

wants to merge 31 commits into from

Conversation

jrconlin
Copy link
Member

@jrconlin jrconlin commented Feb 4, 2015

Add handler for aws DynamoDB storage.

This is using a custom patch for the goamz pending landing goamz goamz/goamz#67
The tests for this package run through a home-grown mock. This mock stubs out the AWS calls in favor of a in-memory checker. This prevents the need to run a secondary server (goamz).
The library will also auto-create the dynamodb tables if not present. The schema includes a global secondary index based off the

This patch is for 1.6+

Conflicts:
	src/github.com/mozilla-services/pushgo/simplepush/aws.go
Ok, it doesn't cache a LOT, but it might reduce some time.
…/dynamo_store

Conflicts:
	Makefile
	src/github.com/mozilla-services/pushgo/simplepush/mock_proprietary_ping_test.go
	src/github.com/mozilla-services/pushgo/simplepush/mock_socket_test.go
	src/github.com/mozilla-services/pushgo/simplepush/mock_storage_test.go
	src/github.com/mozilla-services/pushgo/simplepush/mock_worker_test.go
(needs test)
* DRY the tests
* Convert some strings to CONST labels
* Added DropAll and test (for APNs work)
* changed "created" tag to "modified"
** modified modification modifiers to modify "modification" on
modification.
done := make(chan bool)
timeout := time.After(s.statusTimeout)
go func() {
for {
Copy link

Choose a reason for hiding this comment

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

What does the select statement and <-done do?

Copy link
Member Author

Choose a reason for hiding this comment

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

It performs a loop (with the time.Sleep below). I'll grant that it's a little confusing with the timeout "done" below (which is triggered on a close).

This function was copied from the dynamodb_test lib.
https://github.com/goamz/goamz/blob/master/dynamodb/dynamodb_test.go#L102

Copy link

Choose a reason for hiding this comment

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

Oh, I see...done is unbuffered, so the send on line 105 will block until it's matched by a receive on line 113. Otherwise, if the receive times out, done will be closed, signaling the loop to exit on the next turn. That took a few tries to parse correctly. 😁

@bbangert bbangert added this to the v1.6 milestone Feb 9, 2015
@@ -19,7 +19,8 @@ github.com/ianoshen/gomc 7b9f299f292d3dd707fe2749d966968c9bf1e128
github.com/kitcambridge/envconf fdc1968532255bdb56182329cdaa1e5b9aa6a6ac
github.com/smartystreets/goconvey 8298bc7d36389ffd3e57b85d9797850d8c2382a9
github.com/jacobsa/oglematchers 4fc24f97b5b74022c2a3f4ca7eed57ca29083d3e

github.com/jrconlin/goamz/aws 7d8ddc499fd11cfc58fc2f6a4dcc7e0ff6f930ca
github.com/jrconlin/goamz/dynamodb 7d8ddc499fd11cfc58fc2f6a4dcc7e0ff6f930ca
Copy link
Member

Choose a reason for hiding this comment

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

goamz has applied your patches, we should switch back to their repo.

WriteProv int64 `toml:"write_cap_units" env:"write_cap_units"`

// Region that the dbstore is in
Region string `toml:"region" env:"region"`
Copy link
Member

Choose a reason for hiding this comment

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

toml/env shouldn't be needed since the name is identical.

Access: "MISSING_ACCESS_KEY",
Secret: "MISSING_SECRET_KEY",
ReadProv: 1,
WriteProv: 1,
Copy link
Member

Choose a reason for hiding this comment

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

I thought minimum write/read units were 10/50 respectively. Will AWS just up it to that if you say 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, 1/1 is a lot less than the free tier of 25/25. I was using that as a config mostly because I was just doing simple function testing. I'll bump these values to the free provisioning layer.

Copy link
Member

Choose a reason for hiding this comment

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

Ah! No worries, it can be left there. I had thought the min. was 10.

@bbangert
Copy link
Member

bbangert commented Mar 4, 2015

r- pending some nits, doc changes, panic condition, dynamodb operation changes.

@bbangert
Copy link
Member

bbangert commented Mar 4, 2015

One other note, every read command by default is eventually consistent, we need strongly consistent, which is an additional flag to add to every read/query request.

* Sadly, batch delete prevents meaningful user tests.
// value (it only updates if the previous value matches)
// this is a problem for new fields or running statelessly.
// It does mean that there's a chance of a delayed packet decrementing
// a version or otherwise causing an issue.
Copy link
Member

Choose a reason for hiding this comment

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

Nope. 😁

Doh

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants