Wednesday, March 28, 2012

Is there a more efficient way to perform this process?

My e-commerce site is currently running the following process when items are shipped from the manufacturer:

1. Manufacturer sends a 2-column CSV to the retailer containing an Order Number and its Shipping Tracking Number.

2. Through the web admin panel I've built, a retail staff member uploads the CSV to the server.

3. In code, the CSV is parsed. The tracking number is saved to the database attached to the Order Number.

4. After a tracking # is saved, each item in that order has its status updated to Shipped.

5. The customer is sent an email using ASPEmail from Persits which contains their tracking #.

The process seems to work without a problem so long as the CSV contains roughly 50 tracking #'s or so. The retailer has gotten insanely busy and wants to upload 3 or 4 thousand tracking #'s in a single CSV, but the process times out, even with large server timeout values being set in code. Is there a way to streamline the process to make this work more efficiently? I can provide the code if that helps.

It really depends on how your database backend works. SQL Server? Transactions?

|||

SQL Server 2000. I am not using Transactions. Any suggestions you have as to how I can improve the performance he would be appreciated. Here's what the code looks like. Basically I am building a giant SQL string (multiple EXEC calls on the same procedure) with a stringbuilder and executing it all at once at the bottom:

objOleDb.Open()
objCmd = New OleDbCommand("SELECT * FROM TrackingImport.csv", objOleDb)
objRdr = objCmd.ExecuteReader()

While objRdr.Read()
If Len(Trim(objRdr(1).ToString)) > 0 And Len(Trim(objRdr(0).ToString)) > 0 Then
If IsNumeric(objRdr(0)) Then
If CLng(objRdr(0)) > 0 Then
strSQL.Append("EXEC spImportTracking '" & objRdr(1) & "'," & objRdr(0) & ";")
EmailTracking(objRdr(0), objRdr(1))
i = i + 1
End If
End If
End If
End While

'Update orders with tracking numbers
dbobj.Open()
objSQLCmd = New SqlCommand(strSQL.ToString(), dbobj)
objSQLCmd.ExecuteNonQuery()

The stored procedure called above looks like this:

AlterPROCEDUREspImportTracking

@.TrackingNumberNVARCHAR(50)=NULL,

@.OrderNumberINT=NULL

AS

DECLARE@.OrderAddressIDINT,

@.OrderIDINT,

@.OrderItemIDINT,

@.CountryVARCHAR(2)

/*1. Get OrderID*/

SELECT@.OrderID=(SELECTuid

FROMOrders

WHEREOrderNumber=@.OrderNumber)

/*2. Check Country*/

SELECT@.Country=(SELECTTOP1Country

FROMOrderAddresses

WHEREOrderID=@.OrderID

ANDType=3)

IF@.Country='CA'ANDLEFT(@.TrackingNumber,2)='1Z'

BEGIN

SELECT dbo.Orders.uid,dbo.Orders.OrderNumber,dbo.Customers.FirstName,dbo.Customers.LastName,dbo.Customers.EMail,

'Invalid'AS'TrackingNo'

FROM dbo.CustomersINNERJOIN

dbo.OrdersONdbo.Customers.uid=dbo.Orders.CustomerID

WHEREdbo.Orders.OrderNumber=@.OrderNumber

END

ELSE

BEGIN

/*3. Insert to Tracking*/

INSERTINTOOrderTracking(BackOrderFlag,TrackingNumber,TrackingMessage)

VALUES(0,@.TrackingNumber,@.OrderNumber)

/*2. Get all unpaid (full or partial) orders for thiscustomer*/

DECLAREOrder_CursorCURSORFOR

SELECT OrderItems.uid

FROM OrderItems

WHERE OrderItems.OrderID=@.OrderID

OPENOrder_Cursor

FETCHNEXTFROMOrder_Cursor

INTO@.OrderItemID

WHILE@.@.FETCH_STATUS=0

BEGIN

/*2. Mark All Items in this Order as Shipped*/

EXECspUpdateProductionStatus@.OrderItemID,100

FETCHNEXTFROMOrder_Cursor

INTO@.OrderItemID

END

CLOSEOrder_Cursor

DEALLOCATEOrder_Cursor

/*Return Customer information for confirmation Email*/

SELECT dbo.Orders.uid,dbo.Orders.OrderNumber,dbo.Customers.FirstName,dbo.Customers.LastName,dbo.Customers.EMail,@.TrackingNumberAS'TrackingNo'

FROM dbo.CustomersINNERJOIN

dbo.OrdersONdbo.Customers.uid=dbo.Orders.CustomerID

WHEREdbo.Orders.OrderNumber=@.OrderNumber

END

SETNOCOUNTON

The EmailTracking method that gets called for each record looks like this:

PrivateSubEmailTracking(ByVal intOrderNumberAsLong,ByVal strTrackingNoAsString)

Dim objEmailAsNew CEmail()

Dim strBodyAsNewStringBuilder()

Dim dbobjAsNewSqlConnection(ConfigurationSettings.AppSettings("strConn"))

Dim objCmdAsSqlCommand

Dim objRdrAs SqlDataReader

Dim strBillCountryAsString =""

Try

dbobj.Open()

objCmd =NewSqlCommand("spGetShippingAddresses", dbobj)

objCmd.CommandType = CommandType.StoredProcedure

objCmd.Parameters.Add("@.OrderNumber",intOrderNumber)

objRdr = objCmd.ExecuteReader()

objEmail.strFrom = "shipping@.mydomain.com"

objEmail.strFromName = "Shipping Department"

objEmail.strSubject = "Ship Notification, TrackingNumber " & strTrackingNo

strBody.Append(strIntro)

While objRdr.Read

strBillCountry =objRdr("BillCountry").ToString

objEmail.strTo = objRdr("Email").ToString

strBody.Append("<b>Ship To:</b><br>"& objRdr("Recipient").ToString & "<br>" &_

objRdr("Address").ToString &"<br>" & objRdr("City").ToString &"<br>" & objRdr("State").ToString & _

"<br>" &objRdr("Zip").ToString & "<br>" &objRdr("Country").ToString & "<hr>")

IfobjRdr("WSClient").ToString <> ""Then

UpdatePhotoMgr(objRdr("WSClient").ToString,objRdr("PO#").ToString, strTrackingNo)

EndIf

EndWhile

strBody.Append("<p><b>Tracking Number:</b>" & strTrackingNo & "</p>" & _

"<p><b>Reference Order #:</b>" & intOrderNumber & "</p><p>"& _

SetTrackingLink(strTrackingNo))

strBody.Append(strEnd)

objEmail.strMessage = strBody.ToString

If strBillCountry ="US"Or strBillCountry = ""Or (strBillCountry = "CA"And Left(strTrackingNo, 1) = "D")Then

IfLen(objEmail.strTo) > 0Then

objEmail.SendMail()

Else

Response.Write("<script language='javascript'>alert('AddressMissing for Order " & _

intOrderNumber.ToString &"')</script>")

EndIf

EndIf

Catch objErrorAs Exception

lblError.Text = "Error occurred when sending to" & objEmail.strTo & ": " & objError.Message & _

". Please try re-loading the page, or contact technical support with adescription of this problem."

'Notify tech support

Dim objErrAsNew CEmail()

objErr.SendErrorMessage(objError, "Tracking EmailError", objEmail.strTo, Session("CompanyID"),Session("StoreID"))

objErr =Nothing

context.ClearError()

ExitTry

Finally

objRdr.Close()

objCmd.Dispose()

dbobj.Close()

EndTry

EndSub

1 thought I had would be rather than going to the database each time to retrieve the customer info for email purposes, I could grab all the customer info once in a dataset and then filter with a dataview 1 record at a time to send the emails. Not sure if that would help or not.

|||

1. Using the OleDbConnection to read the file is cool, but I think you could just as well just use a plain StreamReader, get each rows and split the string by the comma.

2. Don't use plain SQL strings. An evil CSV file could wipe out the entire database (consider the row "abc,1;DROP TABLE Orders")

3. Execute everything within a transaction. Not only could it help performance, it's also vital since this operation must be atomic. All the orders should be updated, and if something fails, nothing should happen.

4. Use several calls to spImportTracking (properly parametrized) and execute them each with its own ExecuteNonQuery.

5. Treat the email sending as a separate process. It could fail, and the orders need to know this. Add a status column to the Order table to indicate that an email should be sent, and process it afterwards. Don't return anything from spImportTracking.

6. What does spUpdateProductionStatus do? If it's just setting a status the entire cursor block should be replaced with someting like:

UPDATE OrderItems SET ProductionStatus=100 WHERE OrderID = @.OrderID

Extra:

Log each file upload (who, what and when) in a table with some kind of identifier. Store that identfier with each order (e.g add a column called "batchFileId" referring to the log table). It's good for tracking down errors or mistakes and can also use it when sending the emails (e.g send all emails relating to batchFileid=345)

Don't prefix your stored procedures with "sp". That prefix is reserved for system stored procedures.

An alternative would be to import the actual file contents into a table, and then make one single massive update by relating to that table (the import could be done using a DataAdapter, the SqlBulkUpdate class or just plain inserts (what the adapter would do anyway))|||

Thanks for such a thoughtful reply. I don't usually use SQL strings, but figured appending the Exec statements together and calling ExecuteNonQuery once was more efficient than doing in a loop. I will try that as you've suggested. I will test it out with the StreamReader, and yes, #6 makes a lot more sense. The UpdateProductionStatus procedure has a lot of logic it in to determine the order status which I guess I don't need in this case since I know what the status should be already (and the cursor isn't needed either).

|||

The StreamReader thing was really a minor issue, so don't wast too much energy on fixing that. Remember to use transactions, i.e

BeginTransaction

A lot of ExecuteNonQueries

If success then

Commit

Else

Rollback

End If


No comments:

Post a Comment