Really weird SQL query generated by LightSpeed is killing server

I have an entity called JobItem which has a column called JobItemStatusId of type Guid.

The application is setting JobItem.JobItemStatusId to a value and then for some reason LightSpeed is then selecting all JobItems from the database with that value. I have confirmed this with a break point in a custom ILogger. There is no application code in between calling Set(ref _jobItemStatusId, value, "JobItemStatusId") and the bad select.

e.g.
currentJobItem.JobItemStatusId = 'new value';

results in:
SELECT JobItemId, JobItemStatusId FROM JobItem WHERE JobItemStatusId = 'new value'

Unfortunately there are 40,000 rows in the database with that status and LightSpeed selects and loads all of them with predictably bad memory and CPU usage. The SQL server spikes briefly but the web server is at 100% RAM and CPU continuously.

What's the deal?

LightSpeed version is 4.0.291.16700

.NET 3.5

SQL Server 2008

Windows Server 2008 R2

Yep - that sounds odd indeed since assigning a FK value directly would not trigger a database query so something else must be going on. Ive set up a small example project here mirroring what you have described and that does nothing when assigning the FK value however I know your project is a bit more complicated than that.

Are you able to send through a basic repro project which triggers this please so we can debug into what is triggering this. I am guessing a lazy loaded property is being tripped somewhere which triggers the query.

Jeremy

Ok - I think I understand the sequence of events which triggers this now.

You are calling:

currentJobItem.JobItemStatusId = 'new value';

Where "new value" is the Guid of a JobItemStatus instance which is loaded into the current UnitOfWork. We are checking if the object is in the identity map and if it is then it is treated the same as if you had assigned currentJobItem.JobItemStatus = newValuesJobItemStatusInstance; This triggers the lazy load of the collection because we are performing the object wire-up so in effect assigning the id in this case is the same as if you are assigning the object instance.

Are you treating JobItemStatus's as reference data? (e.g. they are probably always going to be in the current UnitOfWork, or at least you cant guarantee they wont be), if so you could do something like this to avoid the wire-up occurring:

// This will fetch out of the identity map so wont trigger a database call
var heldJobItemStatus = unitOfWork.FindById("new value");

// This takes it out of the identity map
unitOfWork.Detach(heldJobItemStatus);

// This will not longer need to perform object wireup since helpJobItemStatus is no longer in the identity map
jobItem.JobItemStatusId = heldJobItemStatusId;

// Put it back in for later use
unitOfWork.Attach(heldJobItemStatus);

Jeremy

JobItemStatus's are reference data, although some are being loaded into the identity cache because I'm getting them by name in earlier queries for an unrelated reason.

I think there are two issues here:

1. I don't think I have ever accessed the JobItemStatus.JobItems collection, and it isn't set to eager load so I'm not sure why LightSpeed feels the need to go back to the database to populate it. Is it just in case I do eventually access it then LightSpeed is making sure it will be correct?

2. Even if LightSpeed did feel it needed that collection to be up to date with the database I don't think it should do it at this point. LightSpeed should mark the collection as dirty and the next time it is needed then at that point LightSpeed goes back to the database. Better to do it just in time in case that data is never used like in this case. If you're worried about consistency then what about having a temporary collection of entities that have been added to it that sits in memory and then when someone eventually uses the collection then LightSpeed gets all the entities from the database and then appends the in memory entities at that point.

Yes it is more complicated than what you're doing at the moment but what LightSpeed is doing now can be a potentially very harmful bug (i.e. selecting 40,000 entities) that is lying in wait and can be triggered by completely unrelated code putting another entity into the identity map.

JB your work around works.

Whoa. Can I get a more generalized explanation of what's going on here? I don't think I'm seeing this problem however I feel like I'm performing the same tasks.

Just to clarify what is occuring here:

1. You have made a wire-up between the JobItem and the JobItemStatus the collection of JobItems on the JobItemStatus needs to be loaded so we can insert the new JobItem into it. Because the collection was still lazy at the time this triggered the database query.
2. We have to load the lazy collection because we need to keep the state of the collection consistent for any later access during the UnitOfWork. Lets say for arguments sake that we did not load the collection because it was lazy. Later on in the UnitOfWork you access that collection (triggering a database query to fetch the lazy members in), your expectation would be that it would contain all of the members found from the database query plus the item you have added earlier via the wire-up in 1.

It is the requirement that the collection accurately reflect the members from both database and local modifications from the UnitOfWork that lead to the lazy load occuring because you have wired the two objects togethor during a UnitOfWork.

I agree that the outcome under this set of circumstances is somewhat unexpected. Primarily this is because you were making an assignment by Id rather than by Object so I think that if you were assigning the object directly this probably would have been more assumed to occur. I will have a look into what is involved in us performing some local state tracking to sit alongside the collections to allow the actual database lazy load to be defered longer which would have avoided the database query in this particular scenario (so we would only need to trigger the load on actual enumeration/access rather than for add/remove operations) however this is a reasonably low level change so we will be wanting to consider this fairly carefully since there are plenty of edge cases.

Thanks for the help with debugging this!

Jeremy

@chadw: Hopefully my previous post helps clarify what was occuring. In a nutshell assigning by Id is treated as assignment by Object for wire-up purposes if the Object associated with that Id is already in the UnitOfWork which then can trigger database queries from lazy loads when wiring up the objects (as per usual behavior).

Jeremy

I tend to use enums instead of reference tables. So for a User: "user.Status = UserStatus.Inactive". This is a bit annoying to do in the designer but hey, I like it.

But does this mean that if I had a "UserStatus" table instead and did a "user.UserStatusId = 1" then it's going to load all my active users into memory?

If you have the UserStatus instance with the Id of 1 loaded into that specific UnitOfWork (rather than just loaded and detached as reference data) and the collection was lazy at the time then yes that would trigger a database load as described above. This is exactly the same as calling user.Status = statusObject;

Jeremy

chadw:

If the UserStatus entity with a primary key of 1 was loaded previously in your code then they will.

The situation is kind of scary because an unrelated change will trigger everything being loaded into memory - the lookup entity going into memory in one place, the foreign key being set in another. It will also only become a problem when you have lots of data in your database - thousands of users will a status of 1 being loaded into memory - so it can sneak up on you once your project is running in production.

Its simple to fix once you know the problem but understanding it and then finding it is difficult. Hopefully LS 5 will have an improvement here.

Mark invoice as paid, OR/M loads every invoice ever paid. Makes total sense. Ugh.

Just a quick update on this thread, we're looking at improving the handling here to be more clever without breaking things for everyone.

We'll have a nightly fix in the next week. I'll update the thread when it's out.

John-Daniel Trask
Co-founder
Mindscape

Any progress on this?

Also, would it be possible for LightSpeed to provide some sort of performance counters? Perhaps UnitOfWork could maintain both "queries executed" as well as a "entities materialized" counts? It would then be up to the app to pump it into Windows performance counters if desired, I guess by hooking an event or Dispose.

We are still working through the edge cases on this - its actually proving to be a fairly complex change so its taking a lot longer than we had originally hoped. Will post an update as soon as this is ready in the nightlies.

The perf counter suggestion sounds pretty good - particularly for monitoring deployed solutions where you wouldn't otherwise fall back on something like using the MiniProfiler to see whats happening on a page request basis.

Thanks!
Jeremy

Exactly. I can get "queries executed" from the SQL Server performance counters, but having it tied to the UOW is interesting. Likewise I can monitor heap and object thrash, but seeing "Entities Materialized" is really helpful. Maybe hang a Statistics class off UnitOfWork so you add things later.

Hi guys,

I have added in a change for this behavior which is now available in the nightly builds. Going forward the Set() call will not trigger a lazy load in cases where entity wireup only needs to insert or remove a specific entity from the collection. Instead those operations will be deferred until the lazy load occurs at a later time which will typically be triggered by a Get() call.

So primarily this will impact the behavior described here but will also defer lazy loading where you are assigning directly by entity.

Jeremy