In our Rails 3.2.13 app (Ruby 2.0.0 + Postgres on Heroku), we are often retreiving a large amount of Order data from an API, and then we need to update or create each order in our database, as well as the associations. A single order creates/updates itself plus approx. 10-15 associcated objects, and we are importing up to 500 orders at a time.
The below code works, but I fear it's not as efficient as it could be in terms of speed. Creating/updating 500 records takes approx. 1 minute!
Below is the code I would like reviewed:
def add_details(shop, shopify_orders)
shopify_orders.each do |shopify_order|
get_order = Order.where(:order_id => shopify_order.id.to_s, :shop_id => shop.id)
if get_order.blank?
order = Order.create(:order_id => shopify_order.id.to_s, :shop_id => shop.id)
else
order = get_order.first
end
order.update_details(order,shopify_order,shop) #This calls update_attributes for the Order
ShippingLine.add_details(order, shopify_order.shipping_lines)
LineItem.add_details(order, shopify_order.line_items)
Taxline.add_details(order, shopify_order.tax_lines)
Fulfillment.add_details(order, shopify_order.fulfillments)
Note.add_details(order, shopify_order.note_attributes)
Discount.add_details(order, shopify_order.discount_codes)
billing_address = shopify_order.billing_address rescue nil
if !billing_address.blank?
BillingAddress.add_details(order, billing_address)
end
shipping_address = shopify_order.shipping_address rescue nil
if !shipping_address.blank?
ShippingAddress.add_details(order, shipping_address)
end
payment_details = shopify_order.payment_details rescue nil
if !payment_details.blank?
PaymentDetail.add_details(order, payment_details)
end
end
end
def update_details(order,shopify_order,shop)
order.update_attributes(
:order_name => shopify_order.name,
:order_created_at => shopify_order.created_at,
:order_updated_at => shopify_order.updated_at,
:status => Order.get_status(shopify_order),
:payment_status => shopify_order.financial_status,
:fulfillment_status => Order.get_fulfillment_status(shopify_order),
:payment_method => shopify_order.processing_method,
:gateway => shopify_order.gateway,
:currency => shopify_order.currency,
:subtotal_price => shopify_order.subtotal_price,
:Subtotal_tax => shopify_order.total_tax,
:total_discounts => shopify_order.total_discounts,
:total_line_items_price => shopify_order.total_line_items_price,
:total_price => shopify_order.total_price,
:total_tax => shopify_order.total_tax,
:total_weight => shopify_order.total_weight,
:taxes_included => shopify_order.taxes_included,
:shop_id => shop.id,
:email => shopify_order.email,
:order_note => shopify_order.note
)
end
So as you can see, we are looping through each order, finding out if it exists or not (then either loading the existing Order or creating the new Order), and then calling update_attributes to pass in the details for the Order. After that we create or update each of the associations. Each associated model looks very similar to this:
class << self
def add_details(order, tax_lines)
tax_lines.each do |shopify_tax_line|
taxline = Taxline.find_or_create_by_order_id(:order_id => order.id)
taxline.update_details(shopify_tax_line)
end
end
end
def update_details(tax_line)
self.update_attributes(:price => tax_line.price, :rate => tax_line.rate, :title => tax_line.title)
end
I've looked into the activerecord-import gem but unfortunately it seems to be more geared towards creation of records in bulk and not update as we require.
Any suggestions on how this code can be improved? How would you go about improving this based on your experience?
Many many thanks in advance.
UPDATE:
The database is performing quite well as it should, 500 orders with 10-15 associated objects each is about 5500-8000 queries. This is completed in about a minute, so it's handling roughly 100 queries per seconds.. robust enough. Just hoping there is a way that we can reduce the amount of queries needed to accomplish the creation/update of all that data.
-
\$\begingroup\$ It's hard to understand what you are actually doing there. Did you monitor what (and how many) database statement get executed when you import an order? That could give you a clue where the time is spend. \$\endgroup\$iltempo– iltempo2013年09月25日 21:00:25 +00:00Commented Sep 25, 2013 at 21:00
-
\$\begingroup\$ @iltempo Thanks for the comment. We are basically consuming an API for data, and then writing that data into our DB. This is done in a background job, but the initial import done when a new user has installed our saas app requires them waiting for it to complete.. this is why we want to "speed up" the process as much as possible. I have updated the above to show the current database activity. \$\endgroup\$Bjorn Forsberg– Bjorn Forsberg2013年09月26日 14:06:19 +00:00Commented Sep 26, 2013 at 14:06
-
\$\begingroup\$ You can adapt it to work with updates: gist.github.com/jackrg/76ade1724bd816292e4e \$\endgroup\$Fernando Fabreti– Fernando Fabreti2014年08月21日 10:47:56 +00:00Commented Aug 21, 2014 at 10:47
1 Answer 1
You can reduce the amount of queries by putting it into a transaction:
ActiveRecord::Base.transaction do
...
end
This wont reduce the amount of queries but will do them all at once which will save it doing the commit step for each time it has to do the query. Note, that if one of them fails, the transaction will normally be rolled back.
Bulk importing is also another method:
https://github.com/zdennis/activerecord-import
This may not work in your situation as you need to know if there is data there already.
You could also improve your code design to check for the order only once, instead of doing find_or_create_by_order_id
, do this at the start and then send it to the other objects,
for example:
order = Order.find_or_create_by_order_id(:order_id => order.id)
order.shipping_line = shopify_order.shipping_lines
order.line_item = shopify_order.line_item
order.fulfillment = shopify_order.fulfillment
order.save!
Class Order < ActiveRecord::Base
has_many :shopping_line
...
-
1\$\begingroup\$ Thanks for the great ideas, adding the transaction reduced the overall processing time by about 50%! Will implement your other suggestions as well once I have a little more time, but this is a great improvement already. Appreciate it. \$\endgroup\$Bjorn Forsberg– Bjorn Forsberg2013年10月01日 21:28:29 +00:00Commented Oct 1, 2013 at 21:28
-
1\$\begingroup\$ Ps. activerecord-import won't work unfortunately but I've since come accross upsert, which may be a good middle road: https://github.com/seamusabshere/upsert \$\endgroup\$Bjorn Forsberg– Bjorn Forsberg2013年10月01日 21:30:20 +00:00Commented Oct 1, 2013 at 21:30
-
\$\begingroup\$ BTW activerecord-import has some limited support for batch updating based on MySQL DUPLICATE KEY, but I think it requires all columns to be updated. \$\endgroup\$mahemoff– mahemoff2016年01月22日 10:52:20 +00:00Commented Jan 22, 2016 at 10:52
-
\$\begingroup\$ Here's the docs on the activerecord-import feature mahemoff mentioned: On Duplicate Key Update \$\endgroup\$Pete– Pete2016年06月30日 21:13:50 +00:00Commented Jun 30, 2016 at 21:13
Explore related questions
See similar questions with these tags.