1
\$\begingroup\$

I have a semi-working project currently and am wondering if I am doing this the wrong way or if there is a more efficient way. I will try to be as general as possible as to keep the point concise.

Any tips, suggestions, articles, ideas, etc. that you have, will help me greatly. I am a one man show essentially when it comes to projects like this and anything to make it more efficient would be great. Background info: So basically to summarize the project and what it accomplishes, I'll do a list:

  1. Log new orders that need to be sent.
  2. Check 3 data sources for any of the items on that order to see if they shipped or cancelled (order items can be shipped from any or all distribution centers)
  3. Update the count of items shipped (a field in log table) and the count of cancelled items (another field)
  4. If the count of items shipped (in log table) is at least one build data object with necessary data, and send to 3rd party API.
  5. If all items are shipped(or cancelled), update record in log table with completed flag

I am sending nearly 100k+ orders a day. Steps 1, 2 and 3 are in a console app called the "Updater" and the rest of the steps are in a console app called the "Sender" both run on their own separate scheduled task.

My concern here is the "Sender" portion. The thing is we have many different "companies" OR "brands". To ease the load on the data base, I queue each company separately.

Here is my entry point

 var x = Stopwatch.StartNew();
 string[] companies = new string[] { "21", "1", "20", "22" };
 string[] sizesWithDashes = GeneralFunctions.GetSizesWithDashesArray();
 List<Task> taskList = new List<Task>();
 var throttler = new SemaphoreSlim(initialCount: 2);
 foreach (string comp in companies)
 {
     string company = comp;
     DataTable data = UtilitySqlAgent.GetIntialNarvarData(comp);
     await throttler.WaitAsync();
     try
     {
         // *This is where we begin our work* \\
         await FactoryEntrance.InitalizeSenderAsync(comp, data, sizesWithDashes);
     }
     finally
     {
        Console.WriteLine($"Company {company} FINISHED");
        throttler.Release();
     }
 }
 await Task.WhenAll(taskList);
 x.Stop();
 Console.WriteLine(x.Elapsed);
 Console.ReadLine();

Now in this next method, there are a couple key things to keep in mind.

  • Im looping through a set of unique records - all of these records will share an "Initalization Model" object which contains all the data I will need This prevents me from making unecessary queries to the
  • I am attempting to process many records at once, due to the sheer amount of data that I need to get through daily.
  • I am building a string filled with any completed records that need to be updated in the database to execute at the end of the process again to avoid database usage
public static async Task InitalizeSenderAsync(string comp, DataTable data, string[] allSizesWithDashes)
{
    int i = data.Rows.Count;
    var throttler = new SemaphoreSlim(initialCount: 100);
    Console.WriteLine($"There are {data.Rows.Count} orders to send narvar");
    InitalizationModel IM = await BuildInitalizationModelAsync(comp, data, allSizesWithDashes);
    List<Task> taskList = new List<Task>();
    string updatequery = "";
    List<string> updateList = new List<string>();
    foreach (DataRow row in data.Rows)
    {
        await throttler.WaitAsync();
        DataRow rowToProcess = row;
        taskList.Add(Task.Run(async () =>
        {
            try
            {
                Console.WriteLine($"STARTING Company {comp}-{rowToProcess["OrderNumber"].ToString()} ");
                string result = await BeginProcessAsync(IM, rowToProcess);
                if (result != "")
                    updateList.Add(result);
            }
            finally
            {
                i = i - 1;
                Console.WriteLine($"RELEASING Company {comp}-{rowToProcess["OrderNumber"].ToString()} - Remaining: {i}");
                throttler.Release();
            }

        }));
    }
    await Task.WhenAll(taskList);
    foreach (string q in updateList)
        updatequery += $"{q};\n";
    UtilitySqlAgent.UpdateCompletedOrder(updatequery);
}

So I'll only show two more of the methods, because they will give you an idea basically of what im doing in all of the other methods.. Essentially at this point, I passed a row from the unique records to a method that will take the row and build the objects I need, it will convert the root object into an XML object, and that will get POSTed to the 3rd party API.. If it meets the criteria, it will get updated in the log table as complete

private static MasterOrderModel BuildMasterOrderModel(DataRow row, InitalizationModel IM)
{
    MasterOrderModel result = new MasterOrderModel();
    result.NarvarID = int.Parse(row["NarvarID"].ToString());
    string CurrentOrderLocateConstraint = LocateFactory.GetCurrentOrderLocateConstraint(IM.LocateIdData.Select(string.Format("NarvarID ='{0}' ", result.NarvarID)));
    result.CompanyNumber = row["CompanyNumber"].ToString();
    result.OrderNumber = row["OrderNumber"].ToString();
    result.ItemsOrdered = int.Parse(row["ItemsOrdered"].ToString());
    result.ItemsShipped = int.Parse(row["ItemsShipped"].ToString());
    result.ItemsCancelled = int.Parse(row["ItemsCancelled"].ToString());
    result.OrderDate = DateTime.Parse(row["OrderDate"].ToString());
    result.CustomerName = row["CustomerName"].ToString();
    result.CustomerNumber = row["CustomerNumber"].ToString();
    result.ImportDate = DateTime.Parse(row["ImportDate"].ToString());
    try
    {
        result.LocateShipmentData = LocateFactory.GetLocateShipmentData(IM.LocateShipmentData, IM.LocateIdData.Select(string.Format("NarvarID ='{0}' ", result.NarvarID)));

    }
    catch (InvalidOperationException IOE)
    {
        result.LocateShipmentData = null;
        Console.WriteLine($"No Locate Shipment Data for order {result.CompanyNumber}-{result.OrderNumber} ");
        if (IOE.Message != "The source contains no DataRows.")
            throw;
    }
    try
    {
        result.LocateOrderedItemsData = IM.LocateOrderedItemsData.Select(string.Format($"ORDER_NUMBER in {CurrentOrderLocateConstraint}"));
    }
    catch (InvalidOperationException IOE)
    {
        result.LocateOrderedItemsData = null;
        Console.WriteLine($"No Locate Ordered Items for order {result.CompanyNumber}-{result.OrderNumber} ");
        if (IOE.Message != "The source contains no DataRows.")
            throw;
    }
    try
    {
        result.WmsShipmentData = IM.WmsShipmentData.Select(string.Format("NarvarID ='{0}' ", result.NarvarID)).CopyToDataTable();
    }
    catch (InvalidOperationException IOE)
    {
        result.WmsShipmentData = null;
        Console.WriteLine($"No Wms Shipment Data for order {result.CompanyNumber}-{result.OrderNumber} ");
        if (IOE.Message != "The source contains no DataRows.")
            throw;
    }
    try
    {
        result.WmsOrderedItemsData = IM.WmsOrderedItemsData.Select(string.Format("Order_Number ='{0}' ", result.OrderNumber));
    }
    catch (InvalidOperationException IOE)
    {
        result.WmsOrderedItemsData = null;
        Console.WriteLine($"No Wms Ordered Items for order {result.CompanyNumber}-{result.OrderNumber} ");
        if (IOE.Message != "The source contains no DataRows.")
            throw;
    }    
    result.CancelledItemsData = IM.CancellationData.Select($"IHH_ORDER_NUMBER = '{result.OrderNumber}'");
    result.BackorderedItemsData = IM.BackorderData.Select($"IHH_ORDER_NUMBER = '{result.OrderNumber}'");
    result.allSizesWithDashes = IM.AllSizesWithDashes;
    result.ItemShipMethods = IM.ShipMethodData.Select($"OrderNumber = '{result.OrderNumber}'");
    result.NarvarRequestModel = BuildNarvarModel(result, row);
    return result;
}
private static NarvarOrderModel BuildNarvarModel(MasterOrderModel MOM, DataRow initialDataRow)
{
    NarvarOrderModel result = new NarvarOrderModel();
    result.Order_Info = new OrderInfoModel();
    result.Order_Info.Order_Number = MOM.OrderNumber;
    result.Order_Info.Order_Date = MOM.OrderDate;
    result.Order_Info.Status = "PROCESSING";
    result.Order_Info.Currency_Code = null;
    result.Order_Info.Attributes = BuildAttributes(MOM.CompanyNumber);
    result.Order_Info.Billing = BuildOrderBillingModel(initialDataRow);
    result.Order_Info.Customer = BuildCustomerInfoModel(initialDataRow);
    result.Order_Info.Shipments = BuildOrderShipmentsModelList(MOM);
    result.Order_Info.Order_Items = BuildOrderItemsModelList(MOM, result.Order_Info.Shipments);
    for (int i = 0; i <= result.Order_Info.Shipments.Count - 1; i++)
    {
        if (result.Order_Info.Shipments[i].Carrier == "LTL")
            result.Order_Info.Shipments.RemoveAt(i);
    }
    return result;
}

As a benchmark, it's taking me about an hour to send more than 2k orders to the API...

\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

Use Dataflow

You could use the ActionBlock from the System.Threading.Tasks.Dataflow package to let it handle the throttling of InitalizeSenderAsync method that you currently handle yourself:

var workerBlock = new ActionBlock<DataRow>
(
   async row =>
   {
       // body...
   },
   new ExecutionDataflowBlockOptions
   {
       MaxDegreeOfParallelism = 100 // <-- throttle
   }
);

foreach (DataRow row in data.Rows)
{
    workerBlock.Post(row); // <-- feed the ActionBlock
}
workerBlock.Complete();
workerBlock.Completion.Wait(); // <-- wait for all row to be processed

Use StringBuilder or Join

foreach (string q in updateList)
  updatequery += $"{q};\n";

Building large strings in this way might harm the performance. If you care for efficiency then StringBuilder.AppendLine or string.Join($"{Environment.NewLine};", updateList") are your best friends.

Questionable data acquisition

catch (InvalidOperationException IOE)

You should try to avoid this kind of no data handling (No Locate Ordered Items for order). I think you have made this to a reusable pattern because you do this for every single use-case. The methods returning data should return either null or empty collections (like IList<T> (this is the preferred convention). Throwing exceptions is always costly and so many of these try/catch blocks make your ode very difficult to read.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you! Very insightful! Will definitely be refactoring with these ideas in mind \$\endgroup\$
    – boo
    Commented Jul 29, 2019 at 13:45
1
\$\begingroup\$

EntryPoint

Your entry point code does not make sense to me. You have prepared for asynchronously launching tasks, throtling maximum 2 tasks concurrently. However, you forgot to add tasks to taskList and use an await rather than Task.Run. You did implement this pattern correctly at InitalizeSenderAsync.

var throttler = new SemaphoreSlim(initialCount: 2);
List<Task> taskList = new List<Task>();
// .. code
await Task.WhenAll(taskList);

Threading

  • API operations that chain async code calls, should use ConfigureAwait(false); in order not to postback to the initially captured synchronization context.
  • InitalizeSenderAsync updates a local variable i = i - 1; from multiple threads; use Interlocked.Decrement(ref i); instead for decrementing an int atomically.
\$\endgroup\$
1
  • \$\begingroup\$ Its possible i may have gotten confused amidst trying to figure out the right way to configure it.. Any who thanks for the insight! this is exactly the kind of things im trying to figure out. \$\endgroup\$
    – boo
    Commented Jul 29, 2019 at 13:46
0
\$\begingroup\$

You mentioned you're hitting the narvar API?

Without having looked to deeply at your code, try to identify if it's a 'bulk' issue, you mentioned the number 200k (does this mean you hit the API 200k times a day?, see if it's rate limited by the narvar API). If this is the case, look for 'bulk import' / 'bulk actions' in the API, maybe it's throttled for your payment plan, etc...

Whenever I hear 'smallish' numbers (like 200k per day), I think network problems. I wouldn't suspect it's due to the performance of your code.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ According to the Narvar team we work with, they said they can handle any load we throw at them. IIRC there is no rate limit but then again I could be wrong. I will shoot an email and ask.. It's likely youre right about the network issue. I didn't really think about that. Do you think it would benefit me to build all the API request objects into a list and then hit the api afterwards? This may not help performance but possibly identify if the network is the issue. \$\endgroup\$
    – boo
    Commented Jul 26, 2019 at 20:10
  • \$\begingroup\$ Yeah, I would try that (if it's not too hard/too much work to modify your code to bundle API requests). Alternatively, you can try to comment out the network request calls (or mock them with a temporary hard-coded response if your code depends on the response), and see if that speeds up the entire application. (If you have a test environment with dummy data) \$\endgroup\$ Commented Jul 26, 2019 at 21:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.