Forum: Ruby on Rails Strange saving problem

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-23 01:41
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
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-10-23 04:37
(Received via mailing list)
On 22 Oct 2008, at 19:41, Shagy Moe 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
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-23 13:46
> 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.
C64e63b70be7dfed8b0742540b8b27e5?d=identicon&s=25 Mark Reginald James (Guest)
on 2008-10-23 22:55
(Received via mailing list)
Shagy Moe 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
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-24 11:37
(Received via mailing list)
> 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.
80e4cb97cae5c8d745f72337d93fd8f2?d=identicon&s=25 none (Guest)
on 2008-10-24 12:15
(Received via mailing list)
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
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-24 14:37
(Received via mailing list)
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?
C64e63b70be7dfed8b0742540b8b27e5?d=identicon&s=25 Mark Reginald James (Guest)
on 2008-10-24 17:22
(Received via mailing list)
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
80e4cb97cae5c8d745f72337d93fd8f2?d=identicon&s=25 none (Guest)
on 2008-10-24 17:48
(Received via mailing list)
^^ exactly!
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-24 19:22
(Received via mailing list)
Well, that works, but I'm still getting errors under heavy load and
randomly under light load.
D3fc5887a2f39f2e0c8989d39ce5e6f9?d=identicon&s=25 Bharat Ruparel (bruparel)
on 2008-10-25 15:42
(Received via mailing list)
Have you dug into your stack trace?  It may be a time-out error that
is causing all this.  Just a suggestion.
Bharat
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-27 17:15
(Received via mailing list)
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.
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-27 18:47
(Received via mailing list)
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.
D188e591eac11021329b8821a5f954c7?d=identicon&s=25 Ar Chron (railsdog)
on 2008-10-27 23:08
What about a find using :first and :order by version descending to limit
what you have to process...
4bb313af528d968c3a35e8208ecd287b?d=identicon&s=25 Shagy Moe (shagymoe)
on 2008-10-28 01:03
(Received via mailing list)
> 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.
80e4cb97cae5c8d745f72337d93fd8f2?d=identicon&s=25 MaD (Guest)
on 2008-10-28 09:03
(Received via mailing list)
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...

hope this helps.
D188e591eac11021329b8821a5f954c7?d=identicon&s=25 Ar Chron (railsdog)
on 2008-10-28 14:03
Shagy Moe 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.
This topic is locked and can not be replied to.