FinalsClub / karmaworld

KarmaNotes.org v3.0
GNU Affero General Public License v3.0
7 stars 6 forks source link

Course Creation broken on production #267

Closed AndrewMagliozzi closed 10 years ago

AndrewMagliozzi commented 10 years ago

course creation does not add a new course to the database on production. Instead it over-writes the meta-data for the last course in the database.

Try for your self or see this screencast I made:

http://screencast.com/t/CIe0wlQWMYn

charlesconnell commented 10 years ago

Looking into it.

AndrewMagliozzi commented 10 years ago

Thanks. I'm available if you need to discuss.

On Jan 10, 2014, at 3:26 PM, Charles Connell notifications@github.com wrote:

Looking into it.

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

btbonval commented 10 years ago

This is probably related to Django's save operation making its own decision to update or create.

I've bumped into this problem a few times before.

On Fri, Jan 10, 2014 at 3:33 PM, Andrew Magliozzi notifications@github.comwrote:

Thanks. I'm available if you need to discuss.

On Jan 10, 2014, at 3:26 PM, Charles Connell notifications@github.com wrote:

Looking into it.

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

— Reply to this email directly or view it on GitHubhttps://github.com/FinalsClub/karmaworld/issues/267#issuecomment-32063282 .

btbonval commented 10 years ago

https://docs.djangoproject.com/en/1.4/ref/models/instances/#how-django-knows-to-update-vs-insert

It checks to see if the Course object already has a primary key field. If so, it will run an update.

This could happen if Course(**fields) is supplied where the supplied fields are not unique, as in, they are already in the database. -Bryan

On Fri, Jan 10, 2014 at 3:39 PM, Bryan btbonval@gmail.com wrote:

This is probably related to Django's save operation making its own decision to update or create.

I've bumped into this problem a few times before.

On Fri, Jan 10, 2014 at 3:33 PM, Andrew Magliozzi < notifications@github.com> wrote:

Thanks. I'm available if you need to discuss.

On Jan 10, 2014, at 3:26 PM, Charles Connell notifications@github.com wrote:

Looking into it.

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

— Reply to this email directly or view it on GitHubhttps://github.com/FinalsClub/karmaworld/issues/267#issuecomment-32063282 .

charlesconnell commented 10 years ago

Fix committed in c34e051. Live update inserted into production. @AndrewMagliozzi we need you to take the notes that you uploaded to http://karmanotes.org/cedarville-university/politics-american-culture-117 and move them to another course. I'm going to hide them for now.

btbonval commented 10 years ago

@charlesconnell nailed this ticket.

Post Mortem The problem is here, traced all the way back to March 2013: https://github.com/FinalsClub/karmaworld/blob/7c6787751541c2fedf20c625f2ae92267779d2fa/karmaworld/apps/courses/views.py#L24

In the Form's class, object is set to Course(). When the system is first started, the Form class has a single Course object which has no primary key. When a user enters a course in the Add Course form, object is used to add a course to the database. Since object has no primary key, Django performs an INSERT. The problem is that object is tied to the Form class. When a subsequent course is added, the Form class is still associating the same Course object as before, but now object has a primary key. Thus each Course Add results in Django performing an UPDATE in the database, using the primary key on object.

The object attribute couldn't be removed, but the docs mentioned setting it to None would default to creating objects. This form style seems to be used for both update and create, while we are only ever using the form for creation. I think we might using the wrong form style, generally speaking, but this fix is pretty solid.

@AndrewMagliozzi , as Charles noted, there are some notes on production which need to be sorted. Create the course(s) for them, then use the admin panel in Notes to assign the notes to the right course. That's probably going to suck, if you need help, I can probably do assignment directly in the database with SQL magic. At any rate, that problem is ancillary to this ticket.

Closing ticket for great justice! Amazing work, Charles!

btbonval commented 10 years ago

Addition to the Post Mortem: Only one course could be added on each system startup since March, 2013. So if production was running without a Django reboot for say 3 months, then only one course could have possibly been created in that 3 month duration. All subsequent course adds during that time would have overridden the one course.

I analyzed the database on production comparing created_at and updated_at timestamps. They are all off by 4 or 5 hours due to time zone issues in #232 . So I looked for timestamp differences between Course creation and Course updation greater than 5 hours and 1 minute. This resulted in five courses which might have been updated since they were created. Of the five, only the one had a different course name than its course slug. I understand that this update process did not change the slug. It is my conclusion that amazingly only one course on production experienced this issue.

That course had a creation date of January 8th or 9th. It is unknown how many courses were created in the past two or three days, because they'd all have overridden the one course. Speculating, @AndrewMagliozzi is the only other person to add courses besides the one fellow who added the Cedarville U course. That course has been restored. Andrew should know how many courses he attempted to add in the past X days and how many courses of his own were lost.

PS updation is a word. I made it up, but neologisms are still logisms.

AndrewMagliozzi commented 10 years ago

Thanks guys. You rock. I'm stepping out tonight but will restore those notes first thing tomorrow.

Cheers, Andrew

On Fri, Jan 10, 2014 at 5:35 PM, Bryan Bonvallet notifications@github.comwrote:

Addition to the Post Mortem: Only one course could be added on each system startup since March, 2013. So if production was running without a server reboot for say 3 months, then only one course could have possibly created in that 3 month duration. All subsequent course adds during that time would have overridden the one course.

I analyzed the database on production comparing created_at and updated_attimestamps. They are all off by 4 or 5 hours due to time zone issues in

232 https://github.com/FinalsClub/karmaworld/issues/232 . So I looked

for timestamp differences between Course creation and Course updation greater than 5 hours and 1 minute. This resulted in five courses which might have been updated since they were created. Of the five, only the one had a different course name than its course slug. I understand that this update process did not change the slug. It is my conclusion that amazingly only one course on production experienced this issue.

That course had a creation date of January 8th or 9th. It is unknown how many courses were created in the past two or three days, because they'd all have overridden the one course. Speculating, @AndrewMagliozzihttps://github.com/AndrewMagliozziis the only other person to add courses besides the one fellow who added the Cedarville U course. That course has been restored. Andrew should know how many courses he attempted to add in the past X days and how many courses of his own were lost.

PS updation is a word. I made it up, but neologisms are still logisms.

— Reply to this email directly or view it on GitHubhttps://github.com/FinalsClub/karmaworld/issues/267#issuecomment-32073637 .

charlesconnell commented 10 years ago

Thanks to @btbonval for collating all our data and verifying that this hasn't been a large problem.