Reduce sql queries for groupname tests.

[dmitchell]

STUD-1039

@adampalay @cpennington please review

[nedbat]

👍

[adampalay]

👍

[nedbat]

I don't quite understand the structure here: Will this function end up removing the user from more than one group? If so, do we want a .save() for each group? Would it work to move the .save() to after the for loop?

[dmitchell]

I was mechanically editing and not thinking as clearly as you :-) I've made the change on the branch I'm progressing for the other fixes.

[cpennington]

I think you could just do user.groups.remove(*user.groups.filter(name__in=groupnames)), which should cut it down to a single remove query (although from the previous comments, there might only be one anyway)

[cpennington]

👍

[dmitchell]

Please review this next chunk of changes: @cpennington @chrisndodge Refactored loc mapper to use SON for subdocument queries and thus the default '_id' index. (I meant to put these on a different branch building on this one, mea culpa). As far as I can tell, this reduces the mongo time by about a factor of 2. I still need to implement memoization to get rid of most of the calls (I'll do that on a branch of this one this time).

Need to have @e0d add 2 more indexes to location_map on all mongo dbs:
ensureIndex({​'_id.org': 1, '_id.course': 1}{ background:true })
ensureIndex({​'course_id': 1}{ background:true })

[chrisndodge]

@dmitchell looking. I see test failed, but it's a locked database error. Spurious error?!?

[chrisndodge]

Just a clarification: as a high level summary, this just makes sure the queries are hitting the indicies (thus the perf increase). This doesn't reduce # of round trips. Right?

[dmitchell]

Yes, the error looked unrelated to me too.

Yes, this just ensures that every query uses the indices. I'm going to impl memoization to ensure the queries don't repeat which will curtail the round trips.

[chrisndodge]

re: memoization, we have in the code base a "request_cache", which is a cache which has a lifetime during the span of a request. I guess it should have been named "request_scoped_cache" :-) That might be helpful for you to do this.

[chrisndodge]

Can we ding the branch again to get a clean test pass?

+1 but you should get @cpennington to go over it as well

[adampalay]

@dmitchell , that first commit ( 01e66ec) is already in release. #1906

[cpennington]

👍

[dmitchell]

Changed to PR against master https://github.com/edx/edx-platform/pull/1954

[adampalay]

@dmitchell we still want this against release because we're hotfixing off of this PR