Lessig2016 / pledgeservice

GNU Affero General Public License v3.0
9 stars 8 forks source link

Make query more efficient or move to async task #15

Open furf opened 9 years ago

furf commented 9 years ago

We urgently need a performance upgrade to the following endpoint (r/states) that calculates funds raised in each state. It currently times out. We would like a fix to be ready in advance of a press release which will go out on Wednesday.

https://github.com/Lessig2016/pledgeservice/blob/master/backend/handlers.py#L751-L774

tannewt commented 9 years ago

How many requests do you get to that endpoint? It may be worth updating the totals every pledge rather than every request to that page. If it only gets a few hits then you could increase the timeout. Or you could also cache the page entirely.

furf commented 9 years ago

Both are good suggestions, however the query as it stands is inefficient and will not even run the first (pre-cached) time. memcache, as implemented, will probably be an adequate caching solution, but first we need to optimize the call so that it will complete its execution.

brafkind commented 9 years ago

This should help https://github.com/Lessig2016/pledgeservice/pull/16

brafkind commented 9 years ago

Doing an in-memory join from two huge lists (users and pledges) is hugely inefficient. Much better would be to join these data sets in the db. Unfortunately, the DB Datastore API lacks join support. Would we be able to use Google Cloud SQL instead which would support db joins?

aaronlifshin commented 9 years ago

This is High Replication Datastore, not DB. It cannot be queried this way in a web request.

This needs to run in a queue, possibly paginated, write the results out every 10 minutes or so.

A

On Mon, Sep 14, 2015 at 9:19 PM, brafkind notifications@github.com wrote:

Doing and in-memory join from two huge lists (users and pledges) is hugely inefficient. Much better would be to do join these data sets in the db. Unfortunately, the DB Datastore API https://cloud.google.com/appengine/docs/python/datastore/ lacks join support. Would we be able to use Google Cloud SQL https://cloud.google.com/appengine/docs/python/cloud-sql/ instead which would support db joins?

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140246629 .

brafkind commented 9 years ago

Maybe we should rethink either the choice of an HRD DB or the choice of splitting the state info from the pledge model since having to join them using a queue is a huge pain. Unfortunately, switching to a relational db or combining all the state info with the pledges would probably be hard to do by Wed, but perhaps one or the other could be a longer term goal.

aaronlifshin commented 9 years ago

The same query, when running in a server-side process, will have a lot longer to run. Look up gae timeouts.

A

On Mon, Sep 14, 2015 at 9:32 PM, brafkind notifications@github.com wrote:

Maybe we should rethink either the choice of an HRD DB or the choice of splitting the state info from the pledge model since having to join them using a queue is a huge pain. Unfortunately, switching to a relational db or combining all the state info with the pledges would probably be hard to do by Wed, but perhaps one or the other could be a longer term goal.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140248080 .

brafkind commented 9 years ago

Aaron, by "same query" do you mean as a MySQL (relational) query? If that's a concern, then how about at least de-normalizing the pledges and state info so that it's no longer necessary to join with the users?

tannewt commented 9 years ago

Why isn't my suggestion of replacing "model.User.all()" with "db.Query(model.User, projection=('state', ))" enough for Wednesday? It'll limit the user data loaded to just what's used rather than all user fields. I bet the speedup will be enough in the short term. On Mon, Sep 14, 2015 at 6:37 PM brafkind notifications@github.com wrote:

Aaron, by "same query" do you mean as a MySQL (relational) query? If that's a concern, then how about at least de-normalizing the pledges and state info so that it's no longer necessary to join with the users?

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140248727 .

brafkind commented 9 years ago

I think you'd need to project with both the email and state fields so that we can still filter the pledges with each user's email.

On Mon, Sep 14, 2015 at 9:49 PM, Scott Shawcroft notifications@github.com wrote:

Why isn't my suggestion of replacing "model.User.all()" with "db.Query(model.User, projection=('state', ))" enough for Wednesday? It'll limit the user data loaded to just what's used rather than all user fields. I bet the speedup will be enough in the short term. On Mon, Sep 14, 2015 at 6:37 PM brafkind notifications@github.com wrote:

Aaron, by "same query" do you mean as a MySQL (relational) query? If that's a concern, then how about at least de-normalizing the pledges and state info so that it's no longer necessary to join with the users?

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140248727

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140250396 .

tannewt commented 9 years ago

Ah maybe. I'd have to read the docs. You don't need the email server side though. On Mon, Sep 14, 2015 at 6:51 PM brafkind notifications@github.com wrote:

I think you'd need to project with both the email and state fields so that we can still filter the pledges with each user's email.

On Mon, Sep 14, 2015 at 9:49 PM, Scott Shawcroft <notifications@github.com

wrote:

Why isn't my suggestion of replacing "model.User.all()" with "db.Query(model.User, projection=('state', ))" enough for Wednesday? It'll limit the user data loaded to just what's used rather than all user fields. I bet the speedup will be enough in the short term. On Mon, Sep 14, 2015 at 6:37 PM brafkind notifications@github.com wrote:

Aaron, by "same query" do you mean as a MySQL (relational) query? If that's a concern, then how about at least de-normalizing the pledges and state info so that it's no longer necessary to join with the users?

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140248727

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140250396

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140250617 .

brafkind commented 9 years ago

The email address is the foreign key from the pledge to the user. How else would you know which pledges goes with which states?

On Mon, Sep 14, 2015 at 10:02 PM, Scott Shawcroft notifications@github.com wrote:

Ah maybe. I'd have to read the docs. You don't need the email server side though. On Mon, Sep 14, 2015 at 6:51 PM brafkind notifications@github.com wrote:

I think you'd need to project with both the email and state fields so that we can still filter the pledges with each user's email.

On Mon, Sep 14, 2015 at 9:49 PM, Scott Shawcroft < notifications@github.com

wrote:

Why isn't my suggestion of replacing "model.User.all()" with "db.Query(model.User, projection=('state', ))" enough for Wednesday? It'll limit the user data loaded to just what's used rather than all user fields. I bet the speedup will be enough in the short term. On Mon, Sep 14, 2015 at 6:37 PM brafkind notifications@github.com wrote:

Aaron, by "same query" do you mean as a MySQL (relational) query? If that's a concern, then how about at least de-normalizing the pledges and state info so that it's no longer necessary to join with the users?

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140248727

.

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140250396

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140250617

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140253059 .

brafkind commented 9 years ago

By the way, we'd need an index to be created with whichever combo of fields we want to project per the section Indexes for projections https://cloud.google.com/appengine/docs/python/datastore/projectionqueries#Python_Indexes_for_projections which states: Projection queries require all properties specified in the projection to be included in a Datastore index https://cloud.google.com/appengine/docs/python/datastore/indexes.

On Mon, Sep 14, 2015 at 10:25 PM, Barry Rafkind barry.rafkind@gmail.com wrote:

The email address is the foreign key from the pledge to the user. How else would you know which pledges goes with which states?

On Mon, Sep 14, 2015 at 10:02 PM, Scott Shawcroft < notifications@github.com> wrote:

Ah maybe. I'd have to read the docs. You don't need the email server side though. On Mon, Sep 14, 2015 at 6:51 PM brafkind notifications@github.com wrote:

I think you'd need to project with both the email and state fields so that we can still filter the pledges with each user's email.

On Mon, Sep 14, 2015 at 9:49 PM, Scott Shawcroft < notifications@github.com

wrote:

Why isn't my suggestion of replacing "model.User.all()" with "db.Query(model.User, projection=('state', ))" enough for Wednesday? It'll limit the user data loaded to just what's used rather than all user fields. I bet the speedup will be enough in the short term. On Mon, Sep 14, 2015 at 6:37 PM brafkind notifications@github.com wrote:

Aaron, by "same query" do you mean as a MySQL (relational) query? If that's a concern, then how about at least de-normalizing the pledges and state info so that it's no longer necessary to join with the users?

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140248727

.

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140250396

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140250617

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140253059 .

brafkind commented 9 years ago

In https://github.com/Lessig2016/pledgeservice/pull/17 , I propose to increase the cache timeframe from 10 min to 60 min

aaronlifshin commented 9 years ago

This would cause the query to fail 6 times less often. But I worry it will still never actually get the results until its moved into a task queue.

A

On Mon, Sep 14, 2015 at 10:54 PM, brafkind notifications@github.com wrote:

In #17 https://github.com/Lessig2016/pledgeservice/pull/17 , I propose to increase the cache timeframe from 10 min to 60 min

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140262531 .

brafkind commented 9 years ago

Does the loop complete now? How long does it take? Perhaps we could keep it cached for the entire day as long as it can complete once before Wed.

On Mon, Sep 14, 2015 at 10:58 PM, Aaron Lifshin notifications@github.com wrote:

This would cause the query to fail 6 times less often. But I worry it will still never actually get the results until its moved into a task queue.

A

On Mon, Sep 14, 2015 at 10:54 PM, brafkind notifications@github.com wrote:

In #17 https://github.com/Lessig2016/pledgeservice/pull/17 , I propose to increase the cache timeframe from 10 min to 60 min

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140262531

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140262884 .

aaronlifshin commented 9 years ago

It cannot complete in a web handler, because there is a deadline on web handlers in appengine.

The error is a DeadlineExceededError https://cloud.google.com/appengine/articles/deadlineexceedederrors?hl=en

It needs to run in a task queue. https://cloud.google.com/appengine/docs/java/taskqueue/?csw=1

A

On Mon, Sep 14, 2015 at 11:00 PM, brafkind notifications@github.com wrote:

Does the loop complete now? How long does it take? Perhaps we could keep it cached for the entire day as long as it can complete once before Wed.

On Mon, Sep 14, 2015 at 10:58 PM, Aaron Lifshin notifications@github.com wrote:

This would cause the query to fail 6 times less often. But I worry it will still never actually get the results until its moved into a task queue.

A

On Mon, Sep 14, 2015 at 10:54 PM, brafkind notifications@github.com wrote:

In #17 https://github.com/Lessig2016/pledgeservice/pull/17 , I propose to increase the cache timeframe from 10 min to 60 min

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140262531

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140262884

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140263068 .

brafkind commented 9 years ago

Ok, but first a dumb question. If we know what the current total is, could we just hard-code it late Tuesday and comment out the loop? Does this need to be a dynamic calculation on Wed?

On Mon, Sep 14, 2015 at 11:04 PM, Aaron Lifshin notifications@github.com wrote:

It cannot complete in a web handler, because there is a deadline on web handlers in appengine.

The error is a DeadlineExceededError https://cloud.google.com/appengine/articles/deadlineexceedederrors?hl=en

It needs to run in a task queue. https://cloud.google.com/appengine/docs/java/taskqueue/?csw=1

A

On Mon, Sep 14, 2015 at 11:00 PM, brafkind notifications@github.com wrote:

Does the loop complete now? How long does it take? Perhaps we could keep it cached for the entire day as long as it can complete once before Wed.

On Mon, Sep 14, 2015 at 10:58 PM, Aaron Lifshin < notifications@github.com> wrote:

This would cause the query to fail 6 times less often. But I worry it will still never actually get the results until its moved into a task queue.

A

On Mon, Sep 14, 2015 at 10:54 PM, brafkind notifications@github.com wrote:

In #17 https://github.com/Lessig2016/pledgeservice/pull/17 , I propose to increase the cache timeframe from 10 min to 60 min

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140262531

.

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140262884

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140263068

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140263689 .

rickhanlonii commented 9 years ago

Hey all!

I don't think a task queue is the best solution for this. There's a better solution available short term and long term that was already mentioned by @tannewt.

Given your data store and models, your best option is to create a PledgePerState model and increment the values for the User's state on each pledge. Then you just need to seed it with the existing data, and return that data directly in the handler.

aaronlifshin commented 9 years ago

This will also work.

On Mon, Sep 14, 2015 at 11:12 PM, Rick Hanlon II notifications@github.com wrote:

Hey all!

I don't think a task queue is the best solution for this. There's a better solution available short term and long term that was already mentioned by @tannewt https://github.com/tannewt.

Given your data store and models, your best option is to create a PledgePerState model and increment the values for the User's state on each pledge. Then you just need to seed it with the existing data, and return that data directly in the handler.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140267538 .

rickhanlonii commented 9 years ago

Here's where you would want to add the PledgePerState increment.

tannewt commented 9 years ago

I still think projection is the simplest short term solution. @brafkind may be right about needing another index but it will be created automatically. I think it may be covered already by "App Engine predefines a simple index on each property of an entity." from https://cloud.google.com/appengine/docs/python/datastore/indexes. The projection will only include 'state' because email is only in the filter. Third bullet here: https://cloud.google.com/appengine/docs/python/datastore/projectionqueries#Python_Limitations_on_projections

The PledgePerState model has complexity around concurrrent writes which rules it out as a short term solution IMO. See https://cloud.google.com/appengine/articles/sharding_counters?hl=en for details.

aaronlifshin commented 9 years ago

I don't think projection will work. It will reduce the size of each requests slightly, but fundamentally still has to hit each record, and I suspect that hitting each record will still exceed the deadline of the web requests.

However, there is a performance improvement there, so could try it.

A

On Mon, Sep 14, 2015 at 11:54 PM, Scott Shawcroft notifications@github.com wrote:

I still think projection is the simplest short term solution. @brafkind https://github.com/brafkind may be right about needing another index but it will be created automatically. I think it may be covered already by "App Engine predefines a simple index on each property of an entity." from https://cloud.google.com/appengine/docs/python/datastore/indexes. The projection will only include 'state' because email is only in the filter. Third bullet here: https://cloud.google.com/appengine/docs/python/datastore/projectionqueries#Python_Limitations_on_projections

The PledgePerState model has complexity around concurrrent writes which rules it out as a short term solution IMO. See https://cloud.google.com/appengine/articles/sharding_counters?hl=en for details.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140273083 .

tannewt commented 9 years ago

Its possible it won't but it should be easy for someone with access to try. It'll save network latency and time spent getting the data off the wire in the server too in addition to query latency. I figure we don't need a Big O speed up given that (I assume) it used to work within the timeout.

Large efficiency gains depend on the underlying BigTable implementation. If email and state end up in a separate index then the query won't need to lookup the entity at all.

aaronlifshin commented 9 years ago

No. It never used to work within the timeout. It was submitted by a volunteer and only ever worked in test, where there is an order of magnitude fewer entries (like 150 vs 9000).

I've worked in GAE a little bit and these sort of queries (anything iterating a whole table like that) always time out in webhandlers.

A

On Tue, Sep 15, 2015 at 1:24 AM, Scott Shawcroft notifications@github.com wrote:

Its possible it won't but it should be easy for someone with access to try. It'll save network latency and time spent getting the data off the wire in the server too in addition to query latency. I figure we don't need a Big O speed up given that (I assume) it used to work within the timeout.

Large efficiency gains depend on the underlying BigTable implementation. If email and state end up in a separate index then the query won't need to lookup the entity at all.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140285555 .

tannewt commented 9 years ago

Ah, ok. Then yeah, I agree projection probably won't work. Worth a shot though I'd think.

On Mon, Sep 14, 2015 at 10:31 PM Aaron Lifshin notifications@github.com wrote:

No. It never used to work within the timeout. It was submitted by a volunteer and only ever worked in test, where there is an order of magnitude fewer entries (like 150 vs 9000).

I've worked in GAE a little bit and these sort of queries (anything iterating a whole table like that) always time out in webhandlers.

A

On Tue, Sep 15, 2015 at 1:24 AM, Scott Shawcroft <notifications@github.com

wrote:

Its possible it won't but it should be easy for someone with access to try. It'll save network latency and time spent getting the data off the wire in the server too in addition to query latency. I figure we don't need a Big O speed up given that (I assume) it used to work within the timeout.

Large efficiency gains depend on the underlying BigTable implementation. If email and state end up in a separate index then the query won't need to lookup the entity at all.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140285555

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140286476 .

brafkind commented 9 years ago

By the way, I also noticed that the code makes a couple of funny assumptions that probably ought to be corrected, but likely do not impact performance. First, it treats any pledge above $250 as $250. Second, it caps the pledge total per state at $5k (that would allow for a max of $5k/state x 50 states = $250k)

On Tue, Sep 15, 2015 at 2:27 AM, Scott Shawcroft notifications@github.com wrote:

Ah, ok. Then yeah, I agree projection probably won't work. Worth a shot though I'd think.

On Mon, Sep 14, 2015 at 10:31 PM Aaron Lifshin notifications@github.com wrote:

No. It never used to work within the timeout. It was submitted by a volunteer and only ever worked in test, where there is an order of magnitude fewer entries (like 150 vs 9000).

I've worked in GAE a little bit and these sort of queries (anything iterating a whole table like that) always time out in webhandlers.

A

On Tue, Sep 15, 2015 at 1:24 AM, Scott Shawcroft < notifications@github.com

wrote:

Its possible it won't but it should be easy for someone with access to try. It'll save network latency and time spent getting the data off the wire in the server too in addition to query latency. I figure we don't need a Big O speed up given that (I assume) it used to work within the timeout.

Large efficiency gains depend on the underlying BigTable implementation. If email and state end up in a separate index then the query won't need to lookup the entity at all.

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140285555

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140286476

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140292728 .

brafkind commented 9 years ago

Do we have the existing data we'd need to seed such a new model?

I'd suggest optimizing by loading batches of users in memory to reduce the

of db queries which are probably the bottleneck.

On Mon, Sep 14, 2015 at 11:12 PM, Rick Hanlon II notifications@github.com wrote:

Hey all!

I don't think a task queue is the best solution for this. There's a better solution available short term and long term that was already mentioned by @tannewt https://github.com/tannewt.

Given your data store and models, your best option is to create a PledgePerState model and increment the values for the User's state on each pledge. Then you just need to seed it with the existing data, and return that data directly in the handler.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140267538 .

brafkind commented 9 years ago

See PR https://github.com/brafkind/pledgeservice/pull/1 "Load users in batches of 100 to reduce db latency"

aaronlifshin commented 9 years ago

The purpose of this code is to display the qualifying amount for presidential federal matching funds. To qualify, we cannot count more than 250 from any donation, and have to reach the 5000 number in 20 states.

Why limit the total at 5000 I don't know, though...

A

On Tue, Sep 15, 2015 at 6:58 AM, brafkind notifications@github.com wrote:

By the way, I also noticed that the code makes a couple of funny assumptions that probably ought to be corrected, but likely do not impact performance. First, it treats any pledge above $250 as $250. Second, it caps the pledge total per state at $5k (that would allow for a max of $5k/state x 50 states = $250k)

On Tue, Sep 15, 2015 at 2:27 AM, Scott Shawcroft <notifications@github.com

wrote:

Ah, ok. Then yeah, I agree projection probably won't work. Worth a shot though I'd think.

On Mon, Sep 14, 2015 at 10:31 PM Aaron Lifshin <notifications@github.com

wrote:

No. It never used to work within the timeout. It was submitted by a volunteer and only ever worked in test, where there is an order of magnitude fewer entries (like 150 vs 9000).

I've worked in GAE a little bit and these sort of queries (anything iterating a whole table like that) always time out in webhandlers.

A

On Tue, Sep 15, 2015 at 1:24 AM, Scott Shawcroft < notifications@github.com

wrote:

Its possible it won't but it should be easy for someone with access to try. It'll save network latency and time spent getting the data off the wire in the server too in addition to query latency. I figure we don't need a Big O speed up given that (I assume) it used to work within the timeout.

Large efficiency gains depend on the underlying BigTable implementation. If email and state end up in a separate index then the query won't need to lookup the entity at all.

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140285555

.

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140286476

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140292728

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140353436 .

rickhanlonii commented 9 years ago

The idea of limiting to 5k was so that we didn't expose all of the state totals in case that was information we wanted to protect.

aaronlifshin commented 9 years ago

They are not real, because of the MAX(250) rule anyway, so we don't mind exposing them. Let's not limit to 5k.

A

On Tue, Sep 15, 2015 at 10:25 AM, Rick Hanlon II notifications@github.com wrote:

The idea of limiting to 5k was so that we didn't expose all of the state totals in case that was information we wanted to protect.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140409355 .

rickhanlonii commented 9 years ago

That was discussed here.

aaronlifshin commented 9 years ago

This is a nice step, but the request still would not execute within the deadline.

On Tue, Sep 15, 2015 at 7:52 AM, brafkind notifications@github.com wrote:

See PR brafkind#1 https://github.com/brafkind/pledgeservice/pull/1 "Load users in batches of 100 to reduce db latency"

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140366487 .

brafkind commented 9 years ago

How about incrementing the batch size by an order of magnitude until we make the deadline?

On Tue, Sep 15, 2015 at 10:28 AM, Aaron Lifshin notifications@github.com wrote:

This is a nice step, but the request still would not execute within the deadline.

On Tue, Sep 15, 2015 at 7:52 AM, brafkind notifications@github.com wrote:

See PR brafkind#1 https://github.com/brafkind/pledgeservice/pull/1 "Load users in batches of 100 to reduce db latency"

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140366487

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140410313 .

aaronlifshin commented 9 years ago

It's just... I really ... don't know how else to put it.

My experience with AppEngine shows that we just will not be able to iterate the entire pledge table inside of a single web request.

It need to be in task queue or split in multiple web requests. There is an example of splitting in multiple requests in admin.py in the pledges-export.

A

On Tue, Sep 15, 2015 at 10:29 AM, brafkind notifications@github.com wrote:

How about incrementing the batch size by an order of magnitude until we make the deadline?

On Tue, Sep 15, 2015 at 10:28 AM, Aaron Lifshin notifications@github.com wrote:

This is a nice step, but the request still would not execute within the deadline.

On Tue, Sep 15, 2015 at 7:52 AM, brafkind notifications@github.com wrote:

See PR brafkind#1 https://github.com/brafkind/pledgeservice/pull/1 "Load users in batches of 100 to reduce db latency"

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140366487

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140410313

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140410870 .

brafkind commented 9 years ago

Yeah, the task queue does sound like the way to go, although being new to GAE and not having a good way to test this, it might be hard to get it done by tomorrow.

I still wonder if it's possible to load enough of the user and pledge data in-memory to run within a single web request. I'd guess there's a considerable amount of memory available relative to the size of our data.

As yet another idea, we could reduce the # of calculations necessary in any one web request by choosing just one state to process (selected randomly) and filter the users that way. Then, we'd have to set cache timeouts per state. Thoughts?

On Tue, Sep 15, 2015 at 10:32 AM, Aaron Lifshin notifications@github.com wrote:

It's just... I really ... don't know how else to put it.

My experience with AppEngine shows that we just will not be able to iterate the entire pledge table inside of a single web request.

It need to be in task queue or split in multiple web requests. There is an example of splitting in multiple requests in admin.py in the pledges-export.

A

On Tue, Sep 15, 2015 at 10:29 AM, brafkind notifications@github.com wrote:

How about incrementing the batch size by an order of magnitude until we make the deadline?

On Tue, Sep 15, 2015 at 10:28 AM, Aaron Lifshin < notifications@github.com> wrote:

This is a nice step, but the request still would not execute within the deadline.

On Tue, Sep 15, 2015 at 7:52 AM, brafkind notifications@github.com wrote:

See PR brafkind#1 https://github.com/brafkind/pledgeservice/pull/1 "Load users in batches of 100 to reduce db latency"

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140366487

.

— Reply to this email directly or view it on GitHub <

https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140410313

.

— Reply to this email directly or view it on GitHub < https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140410870

.

— Reply to this email directly or view it on GitHub https://github.com/Lessig2016/pledgeservice/issues/15#issuecomment-140412150 .

evanvolgas commented 8 years ago

Is this issue still important or is it less so because the deadline you mentioned has passed?

It would probably be better to use a different datastore for this. For a request like this denormalization is a great idea. Separating your state from your data tends to backfire and turn caching into something of a four letter word. I'm pretty wary of it in most cases.

That being said, and in a similar vein to what @rickhanlonii and @tannewt were talking about, it seems to me that these metrics are associative over date ranges. You have your total at time T https://github.com/Lessig2016/pledgeservice/blob/master/backend/handlers.py#L789. And you need a new total at time T + a. and Total(T+a) = Total(T) + Total(a)

If you can't move to a different data store in the short run and you have to separate your state from your data then why not, at a minimum, filter Pledge's donation time here https://github.com/Lessig2016/pledgeservice/blob/master/backend/handlers.py#L803 so that exceeds the time key of the last time you ran it? Then you add the delta to the old count and that's your new value. It's not gonna be a miracle for throughput... but it should be a quite a bit faster.

I haven't worked with Google App Engine specifically. But assuming they don't do a full table scan over the pledge table in order to perform that filter, I think this will help. And it may be a little bit faster to get into the field than refactoring this to implement a per-state counter a la @rickhanlonii and @tannewt (which I agree is the best possible solution independent of moving to a different data store, which I would probably do anyway if it were me, but I understand that may not be feasible).