Strange saving problem


#1

I’ve got some code that more or less looks like this:

class Project < ActiveRecord::Base

def create_new_document

document = Document.new

document.attributes = { "project_id" => self.id,
                        "attribute2" => "etc..."}

if document.save
  new_document_page = DocumentPage.new

  new_document_page.attributes = { "document_id" => new_document.id,
                                   "attribute2" => "etc..." }

  new_document_page.save
end

end
end

So, the strange thing is that even though the document.save call is
sometimes failing and is wrapped in an “if” statement, it still tries to
create the document page which ,of course, causes DB error since the
new_document.id is nil. Also, I get “Mysql::Error: Cannot add or update
a child row: a foreign key constraint fails” on the Project update sql
because the document_id is the actual object_id instead of the id. The
update statement is trying to put a value like “-618357298” instead of
the document id (which doesn’t exist since it didn’t get saved.

The reason I’m not wrapping both saves in a transaction is because I’m
using attachment_fu to upload the document to amazon S3 and the save on
the document times out while waiting for the document pages to save. The
pages are also being uploaded to S3, and all this uploading takes time.
I’m trying to take advantage of attachment_fu’s automatic uploading when
calling save instead of pushing this off to a background job.

I’m running load testing by creating background jobs which create
projects and thus documents and pages. Most of the time everything
works fine, but under heavy load I get these errors but also rarely
under light load.

It seems like mysql is telling rails that the row was saved but it
wasn’t. The errors returned in regards to the document pages reference
a document id that doesn’t exist in the database. So, I see id skips in
the document table. For instance, the document page error references
document id 939 and the ids in the document table go 932, 934, 935…

In load testing, I have up to 5 app servers selecting from, inserting
into and updating the database. As I said, the errors are less with
medium load, but also happen randomly under very light load. I’m using
the deadlock_retry plugin which resolved some deadlocking issues I was
having.

Ok, so has anyone experienced something similar? It seems as though I
need to beef up the DB server, but the errors on light load suggest
something else.

Any feedback is much appreciated.

Thanks,

Shagy


#2

On 22 Oct 2008, at 19:41, Shagy M. wrote:

I’ve got some code that more or less looks like this:

I think it would be rather more helpful if you posted your code rather
than something that is more or less your code. The devil can be in the
details (random hint - watch out for files with screwed up line endings)

Fred


#3

I think it would be rather more helpful if you posted your code rather
than something that is more or less your code. The devil can be in the
details (random hint - watch out for files with screwed up line endings)

The only thing I left out was additional attributes that aren’t related
to the problem and just add clutter. As I said, normally everything
works ok, but occasionally not and frequently under heavy load. All of
the upload functionality is handled by attachment_fu.


#4

Shagy M. wrote:

So, the strange thing is that even though the document.save call is
sometimes failing and is wrapped in an “if” statement, it still tries to
create the document page which ,of course, causes DB error since the
new_document.id is nil. Also, I get “Mysql::Error: Cannot add or update
a child row: a foreign key constraint fails” on the Project update sql
because the document_id is the actual object_id instead of the id. The
update statement is trying to put a value like “-618357298” instead of
the document id (which doesn’t exist since it didn’t get saved.

Is new_document.id a typo? It should be document.id.
The fact that you’re getting the object_id does mean that
the object is not a model instance.

Let AR take care of setting the foreign key:

document.pages.create!(:attribute2 => “etc…”) if document.save


Rails Wheels - Find Plugins, List & Sell Plugins -
http://railswheels.com


#5

MRJ is right: leave the id handling to rails. but that goes for both
Page and Document.

document = Document.new(:attribute => ‘value’) # set all attributes
except :id, :project_id, :created_at, :updated_at
project.documents << document

page = Page.new(:attribute => ‘value’) # same thing here
document.pages << page

that’s it. the only thing have to do to make this work is setting your
relationships right:

class Project < ActiveRecord::Base
has_many :documents
end

class Document < ActiveRecord::Base
has_many :pages
belongs_to :project
end

class Page < ActiveRecord::Base
belongs_to :document
end

hope this helps
dominik


#6

Is new_document.id a typo?

Yes, sorry.

The fact that you’re getting the object_id does mean that
the object is not a model instance.

That’s what I don’t understand. The database is assigning and id and
returning it to rails. Rails is then trying to use this id to create
the new_document_page, but I get Mysql foreign key errors because that
id doesn’t exist in the database.

Let AR take care of setting the foreign key:
document.pages.create!(:attribute2 => “etc…”) if document.save

I’ll give that a shot, thanks.


#7

The models actually look like this: (notice the “has_one :page”)

class Project < ActiveRecord::Base
has_many :documents
end

class Document < ActiveRecord::Base
has_one :page
belongs_to :project
end

class Page < ActiveRecord::Base
belongs_to :document
end

Interestingly, your suggestions successfully save the document with:

project.documents << document

But the following fails:

document.page << page

However, it is successful when I specify the document.id in the
attributes manually. Could there be a bug with has_one or am I just
doing something wrong?


#8

^^ exactly!


#9

Well, that works, but I’m still getting errors under heavy load and
randomly under light load.


#10

shagymoe wrote:

document.page << page

However, it is successful when I specify the document.id in the
attributes manually. Could there be a bug with has_one or am I just
doing something wrong?

Unlike has_many, has_one isn’t an AssociationCollection, and
doesn’t use the << concat operator. Instead:

document.page = page

or

document.create_page(:attribute1 => value1, …)


Rails Wheels - Find Plugins, List & Sell Plugins -
http://railswheels.com


#11

Have you dug into your stack trace? It may be a time-out error that
is causing all this. Just a suggestion.
Bharat


#12

Well, I’ve narrowed this down to an after_create problem. I’m
versioning the documents and each version depends on the most recent
version created. So there is an after_create method that looks like
this:

##############################################################
class Document < ActiveRecord::Base

belongs_to :projects
has_one :page
acts_as_tree

def after_create
update_version
end

def update_version
self.update_attribute(“version”, self.parent.next_version)
end

def next_version
# This used to be self.children.sort etc… but I wanted to use
the :lock functionality
sorted_subversions = Document.find(:all, :conditions =>
[“parent_id = ?”, self.id], :lock => “FOR UPDATE”)

# If it has no parent, then it is the root and doesn't need a

version.
if self.parent_id = nil
next_version = nil

# If there are children
elsif !sorted_subversions.empty?

 sorted_subversions = sorted_subversions.sort {|x,y|

x.version.to_i <=> y.version.to_i }
latest_sub_version = sorted_subversions[-1] # The last in the
array sorted by version
if sorted_subversions.length == 1 &&
latest_sub_version.version.nil?
next_version = ‘1’
else
next_version = latest_sub_version.version.next
end
else
# If it is not a root, and doesn’t have any children
if self.version
next_version = self.version + ‘.1’
else
next_version = ‘1’
end
end
return next_version
end

end

##############################################################

I removed this after_update method and everything is fine. The
problem is that I’m not sure how to do this without causing the
errors. The record needs to be updated with it’s version after it is
created, depending on what versions already exist in the table. I
tried doing it before, but I was getting duplicate versions if two
documents were saved at the same time. With the :lock option, it
prevents duplicates, but causes the problem described at the beginning
of this thread.

Any help is GREATLY appreciated and thanks for all the help so far.


#13

Don’t like to reply to myself here, but basically I think this is
coming down to the need to lock the entire table while each new
document is being saved. That way no other session can calculate the
same “next_version” until the current record is saved. Is this
possible with transactions? What is the proper way to lock the entire
table in rails? I’m not finding good info on this…

Thanks again.


#14

What about a find using :first and :order by version descending to limit
what you have to process…


#15

what’s solving your problem is a mutual exclusion (called Mutex). you
can try google/wiki to learn more about threading and mutex.

easy to implement:

class Document < ActiveRecord::Base
include ‘Mutex_m’

look here and check out the synchronize method:
http://noobkit.com/show/ruby/ruby/standard-library/mutex_m.html

hope this helps.


#16

What about a find using :first and :order by version descending to limit
what you have to process…

I think two queries at the same time would still return the same
version.


#17

Shagy M. wrote:

I think two queries at the same time would still return the same
version.

Actually I was thinking to simplify, minimizing the time your spending
between the query and the return (you may still need the mutex as
suggested above under heavy load scenarios)

def next_version
next_version = nil
unless self.parent_id.nil?
last = Document.find(:first, :order => ‘version desc’, :conditions
-> [‘parent_id = ?’, self.id])
if last.nil?
next_version = (self.version.nil? ? ‘1’ : self.version + ‘.1’)
else
next_version = (last.version.nil? ? ‘1’ : last.version.next)
end
end
return next_version
end

I think this is an equivalent translation of your code, one query, and
at most 2 tests before the answer is returned. I’m unsure whether the
order will works for your case, but try it out if you’re interested.