Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Team images #3750

Merged
merged 20 commits into from
Sep 11, 2015
Merged

Team images #3750

merged 20 commits into from
Sep 11, 2015

Conversation

chadwhitacre
Copy link
Contributor

Closes #3701.

@chadwhitacre chadwhitacre added this to the Pivot milestone Sep 3, 2015
@chadwhitacre
Copy link
Contributor Author

Includes #3749.

@chadwhitacre
Copy link
Contributor Author

screen shot 2015-09-03 at 12 08 08 am

@chadwhitacre
Copy link
Contributor Author

My thought was that we could store and serve from Postgres, and put a MaxCDN pull zone in front using the main site as an origin server, as we do with assets.

@chadwhitacre
Copy link
Contributor Author

screen shot 2015-09-03 at 12 34 45 am

screen shot 2015-09-03 at 12 36 39 am

screen shot 2015-09-03 at 12 36 07 am

@chadwhitacre chadwhitacre force-pushed the team-images branch 2 times, most recently from 88d24e5 to a44fd0a Compare September 3, 2015 04:51
@chadwhitacre chadwhitacre modified the milestone: Pivot Sep 7, 2015
@chadwhitacre chadwhitacre mentioned this pull request Sep 8, 2015
Closed
@chadwhitacre
Copy link
Contributor Author

@chadwhitacre
Copy link
Contributor Author

It's not unlikely that we'll need an Aspen upgrade in order to work with uploaded files.

@chadwhitacre
Copy link
Contributor Author

Should we use the new FormData functionality to post files via ajax? Doesn't work in IE9.

@chadwhitacre
Copy link
Contributor Author

I'm close to a commit on this. I need to teach the test suite how valid_data can have a file upload in it.

@chadwhitacre
Copy link
Contributor Author

Alright, debugged with @dmk246. The Aspen test client isn't smart enough to set filename and Content-Type for file uploads.

@chadwhitacre chadwhitacre mentioned this pull request Sep 9, 2015
Merged
@chadwhitacre
Copy link
Contributor Author

Serving the images out of Postgres is even slower than I expected. It's taking 2 seconds to serve a 44 kB image locally. O.o

@chadwhitacre
Copy link
Contributor Author

Should we use a push zone instead?

@chadwhitacre
Copy link
Contributor Author

I mean, S3 is standard for this. I was looking to use MaxCDN since we already have an account there.

@chadwhitacre
Copy link
Contributor Author

Okay! It's looking like large objects are sufficiently performant, over against bytea. I have an implementation, and it performs better in one-off tests (ms vs. ~2 s for one request). I'm trying to get some actual benchmarks ...

@chadwhitacre
Copy link
Contributor Author

Needed wrk >= 3.0.3 to pick up wg/wrk#53 (current is 4.0.0). Benchmarks forthcoming ...

@chadwhitacre
Copy link
Contributor Author

filesystem (control)

$ wrk -c1 -t1 http://localhost:8537/assets/banner.jpg
Running 10s test @ http://localhost:8537/assets/banner.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.81ms    1.55ms  41.55ms   98.40%
    Req/Sec   531.80     59.16   595.00     71.00%
  5301 requests in 10.01s, 228.39MB read
Requests/sec:    529.40
Transfer/sec:     22.81MB
$ wrk -c1 -t1 http://localhost:8537/assets/banner.jpg
Running 10s test @ http://localhost:8537/assets/banner.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.82ms    3.26ms  69.33ms   99.28%
    Req/Sec   568.38     66.00   606.00     94.00%
  5664 requests in 10.02s, 244.03MB read
Requests/sec:    565.51
Transfer/sec:     24.37MB
$ wrk -c1 -t1 http://localhost:8537/assets/banner.jpg
Running 10s test @ http://localhost:8537/assets/banner.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.58ms  155.95us   6.63ms   89.52%
    Req/Sec   572.21     45.96   606.00     96.81%
  5398 requests in 10.01s, 232.57MB read
Requests/sec:    539.04
Transfer/sec:     23.22MB
$

bytea (8113217)

$ wrk -c1 -t1 http://localhost:8537/heee/image.jpg
Running 10s test @ http://localhost:8537/heee/image.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.66s    11.49ms   1.67s    50.00%
    Req/Sec     0.00      0.00     0.00    100.00%
  6 requests in 10.10s, 1.57MB read
Requests/sec:      0.59
Transfer/sec:    158.89KB
$ wrk -c1 -t1 http://localhost:8537/heee/image.jpg
Running 10s test @ http://localhost:8537/heee/image.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.64s    39.05ms   1.68s    66.67%
    Req/Sec     0.00      0.00     0.00    100.00%
  6 requests in 10.10s, 1.58MB read
Requests/sec:      0.59
Transfer/sec:    160.45KB
$ wrk -c1 -t1 http://localhost:8537/heee/image.jpg
Running 10s test @ http://localhost:8537/heee/image.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.64s    54.01ms   1.69s    83.33%
    Req/Sec     0.00      0.00     0.00    100.00%
  6 requests in 10.10s, 1.58MB read
Requests/sec:      0.59
Transfer/sec:    160.49KB
$

large objects (4ec58fb)

$ wrk -c1 -t1 http://localhost:8537/haaa/image.jpg
Running 10s test @ http://localhost:8537/haaa/image.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     4.76ms    1.92ms  47.31ms   97.03%
    Req/Sec   207.27     24.00   232.00     82.00%
  2069 requests in 10.02s, 89.35MB read
Requests/sec:    206.42
Transfer/sec:      8.91MB
$ wrk -c1 -t1 http://localhost:8537/haaa/image.jpg
Running 10s test @ http://localhost:8537/haaa/image.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     4.44ms  329.97us   9.92ms   88.79%
    Req/Sec   218.15      7.03   232.00     69.00%
  2176 requests in 10.02s, 93.97MB read
Requests/sec:    217.08
Transfer/sec:      9.37MB
$ wrk -c1 -t1 http://localhost:8537/haaa/image.jpg
Running 10s test @ http://localhost:8537/haaa/image.jpg
  1 threads and 1 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     4.44ms  355.22us   9.57ms   89.96%
    Req/Sec   218.50      6.70   232.00     80.00%
  2179 requests in 10.02s, 94.10MB read
Requests/sec:    217.47
Transfer/sec:      9.39MB
$

@chadwhitacre
Copy link
Contributor Author

method req/sec
filesystem 545.0
bytea 0.6
large objects 213.0

@chadwhitacre
Copy link
Contributor Author

Conclusion: large objects are plenty fast enough to support my original strategy of using the main web app as an origin server, with a MaxCDN pull zone in front of it in production (a la assets).

@chadwhitacre
Copy link
Contributor Author

Processing images introduces complex dependencies. I'm thinking of writing a separate service to do this for us, parking it at www.gratipay-image-processing.com or whatever. You would POST images to it and get back a ZIP file of cropped and resized images. We would use this from both production and development.

@chadwhitacre
Copy link
Contributor Author

Or we can pay someone else to do it: https://www.google.com/search?q=image+processing+as+a+service.

@chadwhitacre
Copy link
Contributor Author

https://6px.io/
https://www.imgix.com/pricing
http://www.blitline.com/

https://www.google.com/trends/explore#q=imgix%2C%20blitline%2C%206px

@chadwhitacre
Make create_github_review_issue more resilient
ed5bde2 
I'm working offline and needed this in order to be able to create teams
offline.
@chadwhitacre
Show Team URL on success in addition to review URL
1696454
@chadwhitacre
Use largeobject instead of bytea for performance
634e126
@chadwhitacre
Show images on Team pages
fbef12b 
This simplifies the /team/image endpoint to not use indirect content
negotiation (via the file extension hack), and, in fact, not to
negotiate at all: we just serve the image we have.
@chadwhitacre
We don't need GIFs, do we?
1741af6 
I looked into WebPs, but they're only supported in Chrome, and SVGs, but
they are tough to secure ("SVGs are not just images but
mini-applications")
@chadwhitacre
Use new gip.rocks service to crop images
18615d9
@chadwhitacre
Dropped GIF
df9906c
@chadwhitacre
Learn about different image sizes: large and small
318c66e
@chadwhitacre
Add a little more error checking for image uploads
f94a5a4
@chadwhitacre
Refine homepage layout now that we have real imgs
4ea4cfa
@chadwhitacre
Save original image for later
03eaf6f 
Who knows what sizes we'll want in the future?
@chadwhitacre
Allow original images to be accessed TTW
b782dc9
@chadwhitacre
Enumerate the Team listings
219ce8b
@chadwhitacre
Fix spacing between details and numbers
28b8b1e
@chadwhitacre
Copy link
Contributor Author

Rebased on master (old HEAD 9415694).

@chadwhitacre
Copy link
Contributor Author

Looks like we can purge individual files from the pull zone cache:

https://docs.maxcdn.com/#purge-cache

@chadwhitacre
Copy link
Contributor Author

screen shot 2015-09-11 at 12 15 04 am

@chadwhitacre
Copy link
Contributor Author

screen shot 2015-09-11 at 12 20 05 am

@chadwhitacre
Copy link
Contributor Author

screen shot 2015-09-11 at 12 26 47 am

@chadwhitacre
Copy link
Contributor Author

I want to get this out tonight. I think I'm going to focus on backfilling images, and reticket the CDN.

@chadwhitacre
Copy link
Contributor Author

Okay! I have images for all approved and under review Teams. I don't think I'm going to bother with images for rejected Teams: I don't like the idea of choosing an image for them since we're not giving them a chance to edit it themselves. Better to just leave them as-is, I think.

@chadwhitacre chadwhitacre mentioned this pull request Sep 11, 2015
Closed
@chadwhitacre
Copy link
Contributor Author

I'm bumping on the CDN.

@chadwhitacre
Copy link
Contributor Author

Now that I have all these images, I want to not have any more Teams created without images, so I don't have to go chase them down anymore. :)

chadwhitacre added a commit that referenced this pull request Sep 11, 2015
@chadwhitacre
Merge pull request #3750 from gratipay/team-images
1e0cc17 
Team images
@chadwhitacre chadwhitacre merged commit 1e0cc17 into master Sep 11, 2015
@chadwhitacre chadwhitacre deleted the team-images branch September 11, 2015 06:18
@Changaco Changaco mentioned this pull request Oct 4, 2015
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Review
Projects
None yet
Milestone
Pivot
Development

Successfully merging this pull request may close these issues.
3 participants
@chadwhitacre @rohitpaulk @sils