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

Put reports in S3; add in process caching #1545

Merged
merged 6 commits into from
May 31, 2016
Merged

Put reports in S3; add in process caching #1545

merged 6 commits into from
May 31, 2016

Conversation

tomwilkie
Copy link
Contributor

No description provided.

@paulbellamy
Copy link
Contributor

I would suggest that, as soon as you have several replicas, the cache hit rate will be very low.

@tomwilkie tomwilkie changed the title Add caching to dynamodb collector Put reports in S3; add in process caching May 31, 2016
@tomwilkie
Copy link
Contributor Author

As discussed, I've put reports into s3 now.

I would suggest that, as soon as you have several replicas, the cache hit rate will be very low.

Agreed. I'll add an (optional) external, shared memcache for reports. I'd prefer to do this in the next PR as there a quite a few outstanding changes. WDYT?

Also, any recommendations re:flags? It feels a bit clunky right now, esp. given we're about to add memcache too.

reportSize = prometheus.NewCounter(prometheus.CounterOpts{
Namespace: "scope",
Name: "report_size_bytes",
Help: "Size of reports written to s3.",

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor Author

@paulbellamy I can haz LGTM?

@paulbellamy
Copy link
Contributor

LGTM

@tomwilkie tomwilkie merged commit 12f2816 into master May 31, 2016
@tomwilkie tomwilkie deleted the dynamodb-cache branch May 31, 2016 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants