nopCommerce 1.7 cache not working?! Possible solution to 1.7 slowness?

thought wrote:
No. Still looking for a solution

nopCommerce team | a.m. wrote:

I can't seem to replicate this a.m.

When you say load a shipping method, do you mean go to shipping rate computation page?

thought wrote:
...you can reproduce it after moving away from caching per request and after starting using static cache (like it was used in nopCommerce 1.60)

thought wrote:
Admin Area > Configuration > Shipping > Shipping Methods. Then go to a shipping method details that you plan to restrict from shipping (step 2)

nopCommerce team | a.m. wrote:

ah, sorry forgot to change caching method.

Going to have a play around.

Andrei,

I was able to replicate the issue as described - you've certainly picked a tricky one to start with :)

The first thing we need to ensure is that we work with just one instance of the entity.

If we implement caching (i'm using the caching provider I previously posted) on the GetShippingMethodById method, when we load the ShippingMethods page, we get each Shipping Method and stick into the cache.

When we click save, we call GetAllShippingMethods() (not cached) which gets shipping methods from our object context. Now we loop through each shipping method in the collection and may call CreateShippingMethodCountryMapping which takes in a ShippingMethod ID rather than the entity itself. This presents a problem since we then call GetShippingMethodById within this method, that returns our cached entity, which we then try and add to our ObjectContext (even though it already contains the entity).

if we take a more domain driven design approach to our repository (Manager) methods, we can help reduce these persistence issues.

I have tried two caching approaches in this example

1) Have one cache dictionary for each entity
2) Cache multiple result sets for each entity

In both implementations I was still experiencing the same issue when trying to re-attach the cached object. The problem seems to be how EF is evaluating entity equality when calling context.IsAttached(). In NHibernate we had to override the GetHashCode and Equals methods on our entity base class so that we could calculate equality by the entity ID. Although this may be done when using EF generated classes, I'm not sure if this is the case for POCOs.

I've asked the question on StackOverflow so will post back when I have an answer.

Unfortunately overriding GetHashcode and Equals methods on our entities does not appear to have any effect on how the object state manager determines whether an entity is already attached.

The only way I was able to get the country restrictions to save was if I checked to see if the shipping method was already attached by passing in an EntityKey:

        public static bool IsAttached(this ObjectContext context, EntityKey key) {
            if(key == null) {
                throw new ArgumentNullException("key");
            }
            ObjectStateEntry entry;
            if(context.ObjectStateManager.TryGetObjectStateEntry(key, out entry)) {
                return (entry.State != EntityState.Detached);
            }
            return false;
        }

...

            var sm = GetShippingMethodById(shippingMethodId);
            if (!context.IsAttached(context.CreateEntityKey("NopEntities.ShippingMethods", sm)))
                context.ShippingMethods.Attach(sm);

However, this did not work at all times and often would result in trying to attach the same entity twice.

In these test I was caching the entire object graph of shipping methods and detaching them from the object context.

I can't afford to spend any more time on this, this week but my thoughts are this.

In order to successfully implement caching with entity framework, we need to first refactor (heavily) how we work with our entities.

In the case of this ShippingMethod example, the process of saving the shipping method involves retrieving the same entity 4 times. We don't need to do this, we already have the entity and its being tracked by our object context.

Instead we should have methods on our entities that allow us to work directly with their collections:

        public void AddRestrictedShippingMethod(ShippingMethod sm) {
            if (!this.HasRestrictedShippingMethod(sm))
                this.NpRestrictedShippingMethods.Add(sm);
        }

        public void RemoveRestrictedShippingMethod(ShippingMethod sm) {
            var shippingMethod = this.NpRestrictedShippingMethods
                .RemoveAll(x => x.ShippingMethodId == sm.ShippingMethodId);
        }

        public bool HasRestrictedShippingMethod(ShippingMethod sm) {
            return (this.NpRestrictedShippingMethods
                .Where(x => x.ShippingMethodId == sm.ShippingMethodId)
                .Any());
        }

Then we just save the entity and the entire object graph is persisted. Plus we only hit the database once!

            //return result;
            Country country = new Country();
            foreach (var shippingMethod in ShippingMethodManager.GetAllShippingMethods()) {
                country.AddRestrictedShippingMethod(shippingMethod);
            }
            ShippingMethodManager.SaveCountry(country);

In fact we only really need a SaveChanges() method on our repository classes so we can persist any changes to our entities.

I think until nopCommerce takes a more DDD approach then we will continue to run into brick walls. Moving to an ORM is a big architectural shift in any application. Unfortunately we are trying to use EF in a very database driven way. To get the real benefit of ORM we need to embrace DDD principles. Let's work with the navigation properties directly rather than having our entities calling our repositories.

Until this is done I can't see caching being an easy thing to implement.

I know that some people are concerned that introducing all these "new" design patterns and principles (TDD, DDD, IoC, DI, ORM - to name a few) will be too much of a learning curve for the community. However (and here's where I get opinionated), if you are a developer you should take the time to learn this stuff and if at the end of it we get an application that performs well, is easy to extend and is testable, then what's the big problem?

nHibernate and EF based projects are very elegant at the design time side of things, but they lose when it comes to scalability in my opinion.

The key principle I think can help here is deciding on the difference between an "application" and a "publication".

Applications must be smart, use short term caching, and change every second...
Publications can be dumb, can use long term caching, and don't change even every day some times...

I know to a hammer everything is a nail, hence all websites are "applications" but that doesn't mean they have to be.

Take object hydration for example.  I was watching my SQL profiler last night and just to load my product detail page was running hundreds of CategoryGetByID procs per page load.  In my circumstance I have 37,000 possible categories, and for the product detail page it's recursing a call to the SQL server zillions of times per page load just to hydrate objects.   This is an outcome that I call a broadcast storm.  It seems simple to call an object, but when that object calls 5 objects who in turn call 1,000 objects who each call 2,000 objects you get an enormous (and unnecessary) load on a database, just to render a static page that should have been "pre-published" rather than generated on the fly every time.

I would put a CDN on the front of it, but even at an origin request, the pages are unreliable at best.

If you dissect the NOP framework, the checkout function is the "application" part but 99% of the site is raw, publications including product detail pages, category pages, and even search results (on a shorter Time to Live).

I am attempting to roll back to 1.6 now, but I may even have to go farther and junk the product and category templates in lieu of more static XSLT transforms or something a bit more reliable.

*sigh*

Jared Nielsen
www.FUZION.org

FUZION wrote:

Compared to what? I would argue that an ORM like NHibernate actually makes an application more scalable.

FUZION wrote:

nopCommerce is an application, no matter which way you look at it. You could use output caching to effectively "prepublish" your pages, but this makes it much harder to control when the cache should be invalidated. What happens when a product price changes or the product goes out of stock. In this situation you need to remove it from the cache - this is why I prefer to use object caching.

Regarding the number of database hits, I agree that this is a colossal amount. However, do not let the current performance put you off using ORMs, we just have some work to do. With features such as lazy loading, batch updates and faster development, there are many benefits over conventional data access methods. We have just completed a large nopCommerce project and I know it would taken considerably longer if we had used <1.60.

Use output cache as said above and invalidate via file dependency on update events.

nopCommerce is a great platform. 1.6->1.7 was a big change, it just needs time to mature.

I just hope this is where the emphasis is before more features come along.

"nopCommerce is an application"

So is Facebook, but I can guarantee you that aside from the asynchronous AJAX callbacks when you post an update, 100% of that website is publication cached content.  There is no possible way that it would survive even the smallest data access load on a per page request basis.  Their recent outage hints at their architecture... only the smallest SQL based error reporting system got exposed to the massive traffic load and brought them crawling to a halt.

http://blogs.wsj.com/digits/2010/09/24/what-caused-facebooks-worst-outage-in-four-years/

I don't have a problem with object caching... I just don't buy into the fact that selecting a row in a table that contains a pre-computed blob of rich information (XML fragment) can possibly be any harder to handle than an object that fires off several hundred queries per object instantiation.  The risk is not in the ORM... it's due to the fact that the object is assembling its data by asking thousands of questions over hundreds of high risk SQL pipes rather than letting the SQL server deliver a set result in one transmission... especially for data that only changes once a month... if that.  It just defeats the purpose of having a set based optimized query set in a database server.  You are assembling a molecule by adding one atom at a time rather than just asking for the molecule.

Handling price and inventory count changes is a separate issue.  On the Yahoo storefront, all product detail data was hard static cache, but a separate (and very low impact) call was made to grab real time price and inventory count.  This gave you the best of both worlds... low resource utilization for the largest and most complex data elements... and real time data availability for only the data that mattered (is it in stock and what is the price right now).

Telling the static part of the product object that it needs to live by the same rules as the dynamic part, particularly when the payload of the bulk of the product data is so huge compared to the dynamic price and inventory data is just crazy talk (my opinion of course :).