Just when it was going well .. locking again

Toddy_BoyToddy_Boy Member Posts: 231
Hi All

Nav 2009 Classic (2.01 Objects)
SQL 2008

I've managed to cure a whole bunch of locking issues in our database by investigation which keys are used when returning data therefore how much data is being locked

Yesterday after no updates having been applied to FOBs locking and performance problems have reared their ugly heads big time. Through monitoring SQL queries this seems to be a very expensive, time consuming and therefore I assume is the problematic query when posting transfer journals
(@P1 varchar(20),@P2 varchar(10),@P3 tinyint,@P4 tinyint,@P5 varchar(10),@P6 varchar(10),@P7 varchar(20),@P8 datetime,@P9 varchar(30))
SELECT TOP 1 NULL FROM "Dynamics Nav 2009 LIVE"."dbo"."aaaa$Item Ledger Entry" WITH (UPDLOCK)   
WHERE (("Item No_"=@P1)) AND (("Variant Code"=@P2)) AND (("Open"=@P3)) AND (("Positive"=@P4)) AND (("Location Code"=@P5)) AND 
(("Bin Code"=@P6)) AND (("Lot No_"=@P7)) AND (("Purchase Date"=@P8)) AND (("Serial No_"=@P9))

I've noticed this strangely does not have an ORDER BY clause thus would table scan and lock a majority the table (bad .. very bad). I can't see any RESETs in the posting routine and can't work out why the locking has returned. Any ideas how to track this down?

Why would
Life is for enjoying ... if you find yourself frowning you're doing something wrong

Best Answer

Answers

  • DenSterDenSter Member Posts: 8,307
    edited 2012-01-26
    So your clue is that there is no ORDER BY clause. This means you have some C/AL commands that generates this query without a SETCURRENTKEY command. You know which fields are filtered from the WHERE clause, from the UPDLOCK hint you know there's a LOCKTABLE in there, and from the SELECT TOP NULL you know that it's an ISEMPTY command. From there it's a matter of finding the C/AL code that generates that query.

    To help solve it, I would start by adding a SETCURRENTKEY for a key that matches the filter criteria.

    <edit>
    Edit for purposes of people that come back in here to research, because this needs a bit more information. My plan when I posted this reply was to take Toddy through this step by step. I wanted to have him check the code, and either find that there already is a SETCURRENTKEY, or add one and find there's STILL no ORDER BY clause. Of course some other people didn't know where I was going and started correcting me for making no sense. Just to make sure this is complete I added this information.

    For most queries, when there is no ORDER BY statement, that means that the C/AL code does not have a SETCURRENTKEY. This works for FIND, FINDFIRST, FINDLAST, FINDSET, but not for ISEMPTY. When there's an ISEMPTY command in C/AL, even when you DO have a SETCURRENTKEY command, the SQL Query will not have an ORDER BY command. So, for ISEMPTY commands, it doesn't matter if you have a SETCURRENTKEY, because it will never end up in the query. You'll have to make sure though, so always check the code and verify the code that you're working with.

    In that case you still need to make sure there's a good key for the filter criteria, and when you have a key that has all the fields in your filter criteria, that is what is called a 'covering index'. There's a link in one of the replies that further explains that concept.
    </edit>
  • Toddy_BoyToddy_Boy Member Posts: 231
    Hi Denster

    Thanks for the reply, can I pick it apart a little ...
    So your clue is that there is no ORDER BY clause. This means you have some C/AL commands that generates this query without a SETCURRENTKEY command. You know which fields are filtered from the WHERE clause,
    Got this, that's the conclusion I came to.
    from the UPDLOCK hint you know there's a LOCKTABLE in there,
    Would the LOCKTABLE suggest that the primary key is to be used overriding the suggested key i.e in this case the Entry No.?
    and from the SELECT TOP NULL you know that it's an ISEMPTY command
    Does this mean the result from the SQL query is an empty recordset?

    I've been trying to hunt the query for a while now ... gonna keep digging, thanks for the pointers
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • Marije_BrummelMarije_Brummel Member, Moderators Design Patterns Posts: 4,262
    Select top 1 NULL does not return a recordset, it does not even read the data from the database.

    Therefore it does not make sense to have an ORDER BY or sETCURRENTKEY

    Try to make a covering index and your query will fly.

    http://dynamicsuser.net/blogs/mark_brum ... dexes.aspx
  • DenSterDenSter Member Posts: 8,307
    edited 2012-01-26
    Toddy Boy wrote:
    from the UPDLOCK hint you know there's a LOCKTABLE in there,
    Would the LOCKTABLE suggest that the primary key is to be used overriding the suggested key i.e in this case the Entry No.?
    No. All you know is that there has to be a LOCKTABLE command, because the query has a UPDLOCK hint. Which index SQL Server is used cannot be distilled from the query itself, that stuff is in the execution plan. You can run the query from a query window and display the execution plan to see which index it wants to use, but that doesn't help you determine which C/AL was used to generate that query.
    Toddy Boy wrote:
    and from the SELECT TOP NULL you know that it's an ISEMPTY command
    Does this mean the result from the SQL query is an empty recordset?
    No. All you know is that the query says "SELECT TOP NULL", which only happens as a result of an ISEMPTY command in C/AL. ISEMPTY is used to determine whether there are any records within the filters.

    Your goal is to figure out where the SQL query is generated, and you use the pieces in the query to determine what the C/AL code looks like.

    No ORDER BY usually means there is no SETCURRENTKEY command, for FIND commands. The exception is ISEMPTY, which won't have an ORDER BY even when there is a SETCURRENTKEY command. This makes sense here, so SELECT TOP NULL with no ORDER BY means ISEMPTY, with or without a SETCURRENTKEY.

    UPDLOCK means there has to be a LOCKTABLE command. So if you find a block of code that wants to retrieve data from that table, but there is no LOCKTABLE command in there, then you know it's NOT the right block of C/AL code.

    SELECT TOP NULL means there has to be an ISEMPTY. So if you find a block of code without a SETCURRENTKEY, with filters on all the right fields, with a LOCKTABLE command, but there is a FINDSET command, then you KNOW that it is NOT the right piece of C/AL code, because a FINDSET results in a SELECT * query

    With those hints is simply a matter of detective work and work your way through all of the code in this user's process. If you want to know which objects to look in, you run code coverage while the user is running the process, and you simply open all of those objects from the object designer to find the culprit.

    Sounds like fun? :mrgreen:
  • DenSterDenSter Member Posts: 8,307
    Therefore it does not make sense to have an ORDER BY or sETCURRENTKEY
    I don't agree with that. It does make sense to have the right index in the query, if only to prevent clustered index scans. A query that targets specific branches in the B tree is always more efficient than a table scan, regardless of whether actual data is retrieved.

    Not saying that covered indexes would not help, but the right SETCURRENTKEY will most definitely help.
  • Toddy_BoyToddy_Boy Member Posts: 231
    Nice article in cover index, for my current problem a cover index is being used consisting of all the elements, however this seems to be being RESET somewhere. Your

    Your article and reply also make sense as to what the ISEMPTY is translated to in SQL (nice), I've started to use ISEMPTY as it seems to have helped performance and locking problems.
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • Toddy_BoyToddy_Boy Member Posts: 231
    Still no joy locating the problem.

    What are the double braces around the field names?
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • DenSterDenSter Member Posts: 8,307
    They're just parentheses around filter criteria.
  • Toddy_BoyToddy_Boy Member Posts: 231
    Does that suggest that criteria has been supplied to those fields?
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • DenSterDenSter Member Posts: 8,307
    Yes. When it says "WHERE (("Item No_"=@P1))&quot; for instance, that means that in C/AL code there was a SETRANGE or SETFILTER command on the "Item No." field.
  • kapamaroukapamarou Member Posts: 1,152
    Since you are using the Classic client you can use the "Client monitor". Start it up with the SQL options, execute your process and then stop it and filter the results to see the SQL commands. Locate your command, remove the filters and read from that line upwards to see which Nav object or C/AL command executed the query.
  • ThomasHej_MSFTThomasHej_MSFT Member, Microsoft Employee Posts: 14
    DenSter wrote:
    Therefore it does not make sense to have an ORDER BY or sETCURRENTKEY
    I don't agree with that. It does make sense to have the right index in the query, if only to prevent clustered index scans. A query that targets specific branches in the B tree is always more efficient than a table scan, regardless of whether actual data is retrieved.

    Not saying that covered indexes would not help, but the right SETCURRENTKEY will most definitely help.

    Avoiding tablescans is always - without doubt - what we want, however adding an ORDER BY clause to a SQL statement is no gurantee that you won't get a table scan.

    Filtering and ordering is two completely different things and SQL server might want to prioritize the "filtering" first and do a simple sort at the end to satisfy the orderby requirement. The reverse is also an option, seeing the ordering matching an index with reasonable statictics could allow and execution plan where the "order by" index was used first and filtering applied to the records read.

    The important think here is to realize, that there is no "simple answer" and no "SQL server will always...", it all depends on the query itself, SQL indexes created, statistical information but also the SQL server version (this has changed over time)...

    SQL server calculates the most optimal execution plan taking into consideration the order by, but if you are filtering on one set of items (global filters, security filters and normal filters) but ordering on something else you have no gurantee of which index gets used.

    This is VERY different from the classic database, where everything was dictated by current key.

    As for the "SELECT TOP 1 NULL" statement, this is - as mentioned - used to satisfy the need to know if one of more records exists as efficiently as possible (no need to return data, just IF they exist).

    What we really would want would be a "SELECT TOP 1 COUNT(*)" but performance measurements proved that the "SELECT TOP 1 NULL" turned out to be faster, which is why we are using this construct.

    The result of the query will be: If records does not exist, an empty resultset. If records exists (no matter how many), we get a single row with a single constant field containing NULL. It would make very little sense to add an "order by" here - The SQL executionplan engine will in all likelyhood ignore any "order by" when the select list does not contain any fields from the table on which the order by operates.

    Most other statements will always have an "order by" clause added which will correspond to the key selected by SETCURRENTKEY - you can't avoid that. Even if you don't set a key, the primary key will be the default selected one.

    Given that the correct key is vital to the classic database and that it will add to the readability of the code, setting a key might actually be the right thing to do although - strictly speaking - is it unnecessary in this scenario...
    Thomas Hejlsberg
    CTO, Architect - Microsoft Dynamics NAV
  • David_SingletonDavid_Singleton Member Posts: 5,479
    Toddy Boy wrote:
    Nav 2009 Classic (2.01 Objects)

    Since ISEMPTY did not exist in 2.01, it means it is code you have added, NOT standard code, so it should be pretty easy to find.
    David Singleton
  • DenSterDenSter Member Posts: 8,307
    Avoiding tablescans is always - without doubt - what we want, however adding an ORDER BY clause to a SQL statement is no gurantee that you won't get a table scan.
    No guarantee, but when there is no sort order in the query, with filters on a long list of fields, and reported slowness, I've seen it work more often than not, and I've done a LOT of these. Let me say it like this: it's the first thing I would try. It's fairly easy to find out where the code is, and it is easy to add a sort order. Hence my initial reply:
    DenSter wrote:
    To help solve it, I would start by adding a SETCURRENTKEY for a key that matches the filter criteria.
    Note that I said "to help solve", and "I would start", I never guaranteed that it would make the issue go away.

    The important think here is to realize, that there is no "simple answer" and no "SQL server will always...", it all depends on the query itself, SQL indexes created, statistical information but also the SQL server version (this has changed over time)...
    Of course, there's never a cookiecutter answer. However, since we only get very limited information on these forums you have to start somewhere, and based on the particular question we try to give people a little push in the back, help them on their way to a resolution. You can't reply to these topics with full dissertations on performance problems.
  • ThomasHej_MSFTThomasHej_MSFT Member, Microsoft Employee Posts: 14
    I think we are on the same page. :D
    Thomas Hejlsberg
    CTO, Architect - Microsoft Dynamics NAV
  • DenSterDenSter Member Posts: 8,307
  • Toddy_BoyToddy_Boy Member Posts: 231
    Over the weekend I ran Update Statistics on a number of key tables which seems to have rectified the problem as there has been no cries of locking today. \:D/ Needless to say this job will now run every weekend.

    The clue came when examining the Query Plan, this showed the Primary key estimated at 50% cost and the key it should have been using estimated at 50% cost, I guess this means SQL selected the primary key to perform the search based on the stats it had.

    As I'm are fairly new to supporting systems at this level in this environment every day is a school day, we don't pay a maintenance fee to our third party so the solution isn't always easy to come by using google. Thankfully there's always help on here. :mrgreen:
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • DenSterDenSter Member Posts: 8,307
    You need to run sp_updatestats every single day, which updates the statistics for all tables with full scan. You should also run sp_createstats 'indexonly' to make sure that your SQL Server has statistics on all index columns.

    If that fixes your problem then this is another example of how all the discussion about various performance related replies in this topic were off the mark, because we didn't cover the essentials.
  • Toddy_BoyToddy_Boy Member Posts: 231
    Sometimes we can't see the wood for the trees. I've now amended the job to run nightly.

    I ran the following but for a number of tables
    update statistics dbo.[aaa$Contract Header]
    
    How does that differ from ..
    sp_updatestats
    
    Also the database is pretty big 75 Gig, should I be worried about the time it would take to create and update the statistics with full scan?
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • DenSterDenSter Member Posts: 8,307
    The first one updates statistics for the "Contract Header" table only. The other one (sp_updatestats) is a standard SQL Server system stored procedure that updates the statistics on all tables in the database.

    The first time that you run it, it'll probably run for quite a while on your database, just because the statistics are all out of date. On a 75GB database though I would be surprised if it will take more than 15-20 minutes after that, because it will only actually do something if they NEED to be updated.

    Just schedule it to run late at night or in the middle of the night.
  • Marije_BrummelMarije_Brummel Member, Moderators Design Patterns Posts: 4,262
    I would not be to sure and happy that the issue is solved forever.

    Your query is on the Item Ledger Entry and it's almost impossible that the distribution of data accross the table changes dramatically during a day or even weeks.

    In most NAV system you don't need to update the statistics very often. Optimising the indexes is more important and this also updates the statistics.

    I would hire a specialist to analyse your system and do reccomendations. This way you'll learn faster and spend your time on other things rather than guessing on google and listening to confusing answers.

    Like Thomas says, SQL is not an engine that "always" does this or that and there are tons of versions and parameters.
  • Toddy_BoyToddy_Boy Member Posts: 231
    Hi Mark

    We've been on SQL since Oct 11 with no maintenance (update stats etc) being ran on the database.

    The Item Ledger Entry gets a lot of action on our system with lots of stock constantly being moved around the business, plus this table has many keys. So am I wrong in assuming (see earlier thread) that the 50/50% cost on key selection pointed to an issue with the statistics on that table?

    It would be nice to have an expert look at the system but when you submit the request for 2 days consultancy and it's turned down there's little you can do but turn to google.
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • Marije_BrummelMarije_Brummel Member, Moderators Design Patterns Posts: 4,262
    My first guess would be that yes, you are wrong.

    By default SQL Server updates the statistics automatically. This is unless you turn it off.

    You can turn it off, but only if you replace it with a manual process. This makes kind of sense since an ERP system does not have constantly changing data distribution.

    An expert would analyse the entire system and the AutoUpdateStatistics and AutoCreateStatistics settings are on the checklist.

    I would go back to my boss if I were you and ask him if he wants to risk his business with this.
  • Toddy_BoyToddy_Boy Member Posts: 231
    Hi Mark

    Both of those are set to True, however when I ran the sp_updatestats and sp_createstats the result in the query window showed stats updated for a whole load of fields.

    It seems more than a coincidence that in running these routines the locking stopped.
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • Marije_BrummelMarije_Brummel Member, Moderators Design Patterns Posts: 4,262
    It seems that you are hard to convince, so I'm going to leave it at this.

    8)
  • Toddy_BoyToddy_Boy Member Posts: 231
    Thanks for input
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • DenSterDenSter Member Posts: 8,307
    If they are having performance issues, then they run update stats and the problem goes away, you simply cannot deny that it helps, you just can't argue with the results. Of course it is not a coincidence that the current complaint goes away if the statistics are so messed up to begin with. I've seen it a number of times that by simply putting in place proper statistics maintenance, that the user's immediate complaints go away.

    Is it the whole answer? Absolutely not, chances are that you will have more performance problems, and when they return, it's NOT going to be because of statistics.

    To begin with, you will also need index maintenance (complete reindex of the entire database on a weekly basis, with a fill factor of 85% is a good start). Maintenance on stats and indexes is the bare minimum of what you need. You will also need to make sure that your infrastructure is up to standard. You should also tune the tables based on actual index usage. You will also have to deal with the programming in a variety of objects, there is a LOT of room for improvement there.

    In my real life job, every time we work on performance problems, in addition to trying to address the immediate issue at hand, we ask the customer to let us do a full system review, to make sure their system is set up properly, and that their database maintenance is up to standard. We make recommendations to improve the situation, and we propose a plan going forward for addressing performance problems (tools, index tuning, code review, etcetera).

    This is Mibuso though, we can't spend this amount of time on a comprehensive performance tuning strategy every time a simple question comes in. Sometimes, simple things fix simple issues. We know there are more problems down the road, but we choose to address those when we get to them.

    It's unfortunate that some people feel the need to make other people look bad to make themselves look better, but that doesn't mean they can't make an excellent point. Getting an expert involved is excellent advise, and you should definitely consider it. If you do it before the real problems start (and they will), you'll be ahead of everyone else, and you'll look like a star.
  • Toddy_BoyToddy_Boy Member Posts: 231
    There have been many performance and locking issuesprior to and since moving to SQL. These are being resolved as and when they occur, plus I amend rogue code whenever I see it. Having SQL has helped me spot locking and rogue queries and thus contributed to help resolve these problems.

    Having the help and advice on this thread from DenSter has resolved this issue at hand. I don't believe for a minute this will be the last of my problems however resolving them is challenging and fun, and when they occur and I can't resolve them I hope to converse again.

    I'd like to know more about this phrase if you have the time.
    complete reindex of the entire database on a weekly basis, with a fill factor of 85% is a good start
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
  • Toddy_BoyToddy_Boy Member Posts: 231
    Cheers, something else to get my teeth into .. everyday's a school day
    Life is for enjoying ... if you find yourself frowning you're doing something wrong
Sign In or Register to comment.