Hi,
Few years back when I opted for XAF I knew very little about how it works so it was hard to predict all of the consequences some of the decisions might have.
One of them is opting for default BaseObject which is based on Oid (Guid) key and by default creates clustered index on the field.
Not to mention I had no idea how really clustered indices worked.
But years have passed, databases have grown and I learned few bits here and there, so here we are.
Recently we had horrible performance at one customer and spent weeks diagnosing it in all kinds of different aspects.
True, customer is using older generation Xeon running Express version of SQL Server and worst of all - still has mechanical disk drives.
But, largest tables barely passed million records and that is along way before hardware should become such bottleneck that certain action that should finish in couple of seconds took - 5 minutes to complete. And the worst part - not always, just sometimes - which just made it harder to diagnose.
And just couple of nights ago I dreamed about clustered indices (yea I know, I need a vacation :-)) and in the morning I questioned my entire reality and thought "How could a clustered index perform with random generated key on mechanical disks". Well, beyond horribly that's how.
Everything is great on smaller databases, but or other customer running Samsungs 850 Pro in RAID is also experiencing slowdowns, although not nearly as our first customer so it did not trigger any alarms.
This particular action does invoice booking which means writing data through several tables and one of them is over million record now.
Then I finally looked index fragmentation and I was appalled with numbers which were mostly in 99% range and on top of the list were all primary keys.
Finally it all made sense. Action triggers chain reaction of moving actual data (since clustered index is the table itself) through several large tables creating huge fragmentation in process. That is why the action took more than 5 minutes sometimes to complete (keep in mind this is action they execute hundreds of times a day when booking invoices for instance).
And sugar on the top of all was - Audit Log. All those actions are logged which at the time of writing created table of 2.5 million records. Which would be fine if each time a new log is created, SQL Server needs to insert a new record sorted by Oid - because of clustered index.
Then I noticed tons of indices on AuditDataItemPersistent which took more than 1GB of space and surely did not help with any CRUD operation in entire application.
I checked the source code and there really are [Indexed] attributes on several columns.
Because we can't go back at this point, we created identity columns in most offending tables and created clustered index over them while keeping primary key on Oids with non-clustered index with some magic-no-one-is-sure-how-exactly-works kind of script :-)
This is just lesser of two evils, but this evil at least allows customers to work.
I know there are benefits of choosing Guid as the key, but you should have done it differently, definitely not allowing clustered indices on random generated column forcing database engine to sort records by this random data creating huge fragmentation and moving data around.
Or - there should be some huge red flashing warning when creating project stating that if you are creating a project that might get larger in data not to choose BaseObject as base for all your classes.
Also, Oids for PK are beneficial in situations like replication - but how many customers actually use replication? My guess is no one with small projects - those who did not notice this issues. And everyone with big projects surely ran into this same wall and had to set things differently anyway and you helped them in no way with Oid columns.
So, why not just go with identity columns in the first place? Smaller project wont get hurt and larger projects will have to deal with specifics anyway, but wont have to deal with this huge issue of having database killing itself because every table is subject to enormous fragmentation and data moving (which BTW is really really bad for SSDs).
And you most definitely should not have use BaseObject on AuditDataItemPersistent, AuditedObject and XPWeakReference - which are MOST write heavy tables in entire system causing tremendous slowdowns on anything but super high-end hardware.
Also, since they are - heavy write - rarely read - kind of tables - you definitely need to remove all those indices and let us choose if we want them and how we want them.
There, I shared my last few weeks of experience which lead to dreaming clustered indices - so go figure :-)
Regards,
Mario
Hi Mario,
we had same Situation some years ago - read this:
https://www.devexpress.com/Support/Center/Question/Details/Q536671/using-newsequentialid-on-sql-server
it is about sequential guids - some operations in our System went down from 5-6 minutes to some seconds…
Hi Martin,
Thanks for the link. Nothing really helps at this point since I am not sure I am willing to change primary keys on live databases at this point, which is why we went with another identity column on which we created clustered index.
And for future project, I ain't touching BaseObject!
What worries me though is your ticket is 4 years old, problem is more than obvious and nothing changed since then :-(
Regards,
Mario
This topic i find very interesting we have a large system that is pretty slow and i have never been able to pin point the issues database is only about 10GB on a high end machine. Could you elaborate further as to how and where you then changed to not use the Oid from BaseObject and what it entailed to change your db.
Thanks
Hi,
First of all, have in mind that this issue is not responsible for ALL slow database performance, especially read performance.
This specific issues (clustered index on random data) causes huge defragmentation of data and huge data moving rising number of data written (which can be a concern on SSDs with high volume databases).
So, this can significantly impact write performance especially inserting data into tables which can cause chain data move. Data has to be moved because if you insert 100 records, they will not end up at the end of your 2.5 million record table, but will be scattered across entire table which can force SQL Server to move data pages to be able to fit those records in specific places.
Now, if you have a process that affect several tables like that then the action that lasts 5 seconds or less can take upto 20 minutes to complete, based on fragmentation and of course hardware performance.
But, getting super-fast SSDs just slightly hides the problem as it can perform data move much quicker than mechanical disks, but you can see the problem in excessive wear.
I would also like to know if Martin actually replaced primary keys on existing database - that is something I did not dare to do at this moment.
What we did is following:
- list most offending tables by using query which displays fragmentation (https://goo.gl/EmesPD or google for other examples)
- keep in mind that this script will show nearly ALL indices are above 99% fragmentation because all tables use clustered index on random generated guid, which is why you need to combine this list with size of tables and your own understanding of how tables are used prioritizing heavy-write tables
- this gave us list of tables and indices which needed to be fixed or removed
- then I wrote a script that did following:
- script listed all tables that are referencing offending tables and remembered those foreign keys
- then those keys were removed which was necessary to drop clustered index
- after FKs were removed, script dropped primary key and clustered index
- then new IDENTITY ID field was created on the table
- clustered index is then created on the new Id field
- primary key is then created with non-clustered index on Oid field
- remembered foreign keys are restored
Keep in mind this is not a solution, just "lessening" down the evil we currently have. This will avoid further fragmentation of table itself and excessive data moving, but will still have fragmented primary key index - but this time, it is non-clustered, so does not affect table, but only index itself.
We also used Ola's script (https://goo.gl/K3i2cm) to create scheduled tasks to more often maintain indices based on fragmentation.
This process is still in testing phase since we cant rollback after we push it to customers (for some additional resons, not important for this topic) - which means I still don't know exact effect regarding performance improvement. For now I can only confirm fragmentation of tables is solved - which should lead to significant increase in write performance especially on lower-end hardware and mechanical drives.
Regards,
Mario
"I would also like to know if Martin actually replaced primary keys on existing database - that is something I did not dare to do at this moment."
no we did not - using sequential guid nearly solved the problem - and we have to use guid's because we use replication for some customers!
So, at one specific moment you just switched to sequential guids? Did you have to specify "starting" guid to avoid any possible duplicates, even at theoretical level?
Thanks,
Mario
Nope - simply start using it, you will not have any collissions
Hi,
If anyone will be interested, I went with Martin's solution and completely dropped my first attempt with identity columns for following reasons:
1) Martin cleared that I did not have to touch primary keys which is why I dismissed his solution at first
2) XPO does not natively support read-only persistent properties at the moment and I was not willing to create decendants of UnitOfWork, Session and ObjectSpace because that would involve changing a lot of old code which is directly using UnitOfWork AND
3) would still need to mess with customer data, even if I would change all that code
So, Martin's solution is far better mostly because of the fact that customers data will remain untouched. It will still involve changing A LOT of database code, but it's manageable especially if it leads to not touching customers data.
Anyway, if you end up in similar situation, go with Martin's solution which includes:
- changing your classes to XPCustomObject (I already had all classes inherited from my custom class, so this was easy) and making sure you have at least one persistent property in your base class so you can differentiate your tables from DevExpress tables in later script.
- creating sequential guid function from mentioned link, I will post C# function from mentioned link here:
[DllImport("rpcrt4.dll", CharSet = CharSet.Ansi, SetLastError = true, ExactSpelling = true)] public static extern int UuidCreateSequential(ref Guid id); public static Guid NewSqlSequentialGuid() { Guid newGuid = default(Guid); int returnVal = 0; returnVal = UuidCreateSequential(ref newGuid); byte[] bytes = newGuid.ToByteArray(); if (returnVal != 0) { throw new Exception("CreateSequentialGUID failed."); } else { Array.Reverse(bytes, 0, 4); Array.Reverse(bytes, 4, 2); Array.Reverse(bytes, 6, 2); newGuid = new Guid(bytes); return newGuid; } }
- create default constraint on all tables from your classes (not touching DevExpress tables used by various modules). I used following script to do that:
declare @tbl nvarchar(200) declare @sql nvarchar(max) declare dbc cursor for SELECT t.name AS 'TableName' FROM sys.columns c JOIN sys.tables t ON c.object_id = t.object_id WHERE c.name = 'YourCustomFieldFromBaseClass' ORDER BY TableName; open dbc fetch next from dbc into @tbl while @@FETCH_STATUS = 0 begin set @sql = 'IF OBJECTPROPERTY(OBJECT_ID(''DF_' + @tbl + '_Oid''), ''IsConstraint'') <> 1 ALTER TABLE ' + @tbl +' ADD CONSTRAINT DF_' + @tbl + '_Oid DEFAULT newsequentialid() FOR Oid' print @sql exec sp_executesql @sql fetch next from dbc into @tbl end close dbc deallocate dbc
- and last but not least - change all your stored procedures that inserted NEWID(). Keep in mind here that you need to remove Oid and NEWID() from INSERT statements because only then will SQL server use DEFAULT constraint. If you leave Oid in insert statement and pass NULL as value - default value will not be inserted.
I still use Ola procedures to rebuild indices after everything is complete.
I have managed to create a conversion script which seems to be working in first tests.
Over the weekend I will try it on customers database in my environment which will be good opportunity to test performance difference.
Regards,
Mario
Small but important correction for SQL script:
-- I forgot ISNULL, without it condition is always false since objectproperty returns null set @sql = 'IF ISNULL(OBJECTPROPERTY(OBJECT_ID(''DF_' + @tbl + '_Oid''), ''IsConstraint''), 0) <> 1 ALTER TABLE ' + @tbl +' ADD CONSTRAINT DF_' + @tbl + '_Oid DEFAULT newsequentialid() FOR Oid'
Hi,
So, we are nearly ready to roll out to customers, internal tests are going well for now.
However, there is one thing I do not like in current solution.
Since we moved to sequential uid, after few days of testing I would like all tables to have same solution.
Currently AuditLog tables and SecuritySystem tables are different (continue using NEWID since they are inherited from BaseObject). I am not that worried about Security tables since they are fairly small with very limited growth potential.
However, AuditLog is serious problem and needs to be fixed.
I absolutely want to avoid recompiling DevExpress sources since that brings whole another level of complexity to maintenance and updates.
So, is there another way to force AuditLog NOT to use BaseObject?
In future versions I would prefer abstract XPCustomObject variant. For instance, AuditDataItemPersistentCustom - so it does not brake code for current users or force them to change their code.
But personally, I think you really should move to sequential guids using reverse method - if not generally then AT LEAST in AuditLog module which is a total app killer after a while with additional indexes on several fields which are used extremely rarely but needs to be updated on each and every single CRUD operation.
Just google "random guid clustered index" if you do not believe me or need more scenarios that this combination is horrible idea.
Regards,
Mario
Mario - simply take the source code of auditdataitem and use your own class instead dx
var auditTrailModule = application.Modules.FindModule(typeof(AuditTrailModule)) as AuditTrailModule;
if (auditTrailModule != null)
auditTrailModule.AuditDataItemPersistentType = typeof(MyAuditDataItemPersistent);
Oh cool, thanks.
What is the best place to place this code?
Application constructor in Win/WebApplication.cs?
Also, is there a way to do the same for XPWeakReference and AuditedObjectWeakReference? They are quite large on our side as well.
Thanks,
Mario
i do it in my base module setup method
public override void Setup(XafApplication application)
i also have a custom AuditedObjectWeakReference - but it is derived from BaseAuditedObjectWeakReference, so it has a "normal" guid - but i guess you can set your seq. guid in the afterconstruction method of your custom AuditedObjectWeakReference
Hm, I am getting "same tablename" exception between my class and AudiItemDataPersistent.
Cant find a way to exclude AuditItemDataPersistent type.
Any idea?
Just realized I wasnt very clear.
I used Persistent("AuditItemDataPersistent") attribute on my class because I want to reuse same table, not create new one.
To XAF team: This needs to be updated: https://documentation.devexpress.com/expressappframework/113154/Frequently-Asked-Questions/Business-Logic/What-data-type-should-I-choose-for-a-primary-key-Integer-vs-GUID
with huge warning about clustered index issue.
Hello Mario,
Thank you for taking the time to share your feedback and solutions you implemented for this particular scenario. The solutions you and Martin outlined look fine. The AuditTrailModule.AuditDataItemPersistentType property is usually set in the Application Designer, though your method is also fine and it can be even more preferable if you develop apps for several platforms. Creating custom persistent classes for the AuditTrail module based on their source in \DevExpress.Persistent\DevExpress.Persistent.BaseImpl is correct too. You can either implement your own classes from scratch or use inheritance. The XPWeakReference class provides a public Oid field of the System.Guid type, so you can override it in your code (e.g., in the overridden AfterConstruction method).
I must note, however, that our AuditTrail module was not originally designed to audit thousands of object changes at once. This implementation technically operates at the UI level, which makes it suitable for SME apps. For complex apps for large enterprises, it is usually better to have this kind of audit on the application server or even at the database level. If you need to cover such high-load scenarios, it is possible that you might want to implement your own fully custom auditing instead of using our built-in module. Depending on your requirements, these custom-tailored solutions may start from creating lightweight custom records with the user information saved in the IObjectSpace events and ending with advanced solutions at the XPO or application server level (e.g., check out the recent discussion at Auditing data access in XAF or see Audit Tables Insert Performance).
As for the BaseObject class key type choice, we shifted from integers to guids about a decade ago. We are also happy that we have recently changed the BaseObject > OidInitializationMode property to AfterConstruction as it has helped to improve customer experience and avoid issues in many scenarios. In short, developers do not need to care about supplying a unique value for a key and it is always supplied for you, and it is helpful not only for replication. The whole topic 'Integer vs GUID PK' is very debatable in the non-XAF community and we, as framework developers, allow our users to easily use any type they really need. Even though we are not yet ready to alter the default BaseObject class behavior at this stage due to the number of reasons, our team will take your experience into account for the future. What we can review right now is the fact that the DCBaseObject and a few others do not currently take into account the XpoDefault.GuidGenerationMode property or do not use the XpoDefault > NewGuid method. We will follow up with you if we have more news in this regard. We will also consider describing this specificity in our learning materials. Thanks.
Dennis,
Your architectural decisions open up very likely potential to cause extremely painful and very costly experiences for having to deal with core database issue in live, production databases.
Our customers are absolutely pissed, seriously considering dropping on us and changing solution. It is that bad.
You HAVE to take that VERY seriously. Words like "consider" do not quite fit well here.
I am doing my best to stay polite as possible while still making sure you understand the gravity of this issue.
I have lost weeks of work, trust of my customers and any potential customers in contact with them, I still struggle implementing solution correctly - all for trusting your architectural decision and not reading a single warning of issues it might cause.
It is my fault not to now better - but that is exactly the reason why I chosen your framework - trusting you do know better.
Also, we do not need audit log to audit thousands and thousands of changes at once - thats not the point at all.
Single action might cause maybe a hundred of records in audit log.
The issue with audit log is that every single next update/insert/delete causes SQL Server to shift data pages in order to fit new record in audit log tables because tables are arranged by random guid.
So, it is not about number of inserts audit log needs to handle (it is low) - it is about number of records in audit tables and the fact they are sorted by Oid because by default table has clustered index over random-generated Guid value.
Regards,
Mario
Is there a way to avoid SameTableNameException between my audit class and default one?
I cant deal with it any more, I went with different table name for my class and will probably move data or something, but would still like to know.
And the issues just keep piling up.
I wanted to make sure that all future tables use sequential guid as well, so I used DbType("uniqueidentifier DEFAULT (newsequentialid())") attribute on Oid in my base class.
However, this fails because each foreign key is then also created with DEFAULT clause - which is incorrect and breaks everything.
Besides running script which will be checking on each new version if there are Oid primary keys without DEFAULT - is there another way to accomplish this?
Hello Mario,
One the most reliable ways would be ensuring that the default classes from the DevExpress.Persistent.BaseImpl library are not added into the list of exported types of your application. To do this, you can take the source code of these classes and implement your own versions in your project + configure the AuditTrailModule.AuditDataItemPersistentType property accordingly.
Another option would be dynamically removing and re-adding the PersistentAttribute to a required member of required persistent classes. However, I would still prefer the first option.
As for the DbTypeAttribute, we also discussed it internally and for now we cannot see a quick solution short of altering your database using scripts.
As for this situation in general, I want to assure you that we take it seriously and will take required actions internally. Documentation and KB updates is just one of the possible options I can voice at this stage. We also always try to save the time of our customers by listing potential development problems or implementation considerations for various scenarios, even if they are not directly related to our products. Let's take How to measure and improve the application's performance or Deployment Tutorial > Deployment Troubleshooting Guide as examples. It is important to understand that neither our documentation nor our product itself can police each and every situation that may happen in development using a wide range of .NET and other involved technologies. This is especially the case when it comes to dealing with different deployment environment specifics, e.g., web server or network configurations or profiling database access. Developer, DBA (and often other specialist) decisions must be taken in such tough cases, depending on the end customer requirements.
Once again, I want to thank you for raising this topic and describing your current solutions to the community. As promised, I will update this thread once we have new info.
> One the most reliable ways would be ensuring that the default classes from the DevExpress.Persistent.BaseImpl library are not added
> into the list of exported types of your application. To do this, you can take the source code of these classes and implement your own
> versions in your project + configure the AuditTrailModule.AuditDataItemPersistentType property accordingly.
This is exactly what I did (see image attached) and when I am in Application Designer I can see my classes only, but for some reasons, somewhere original classes still end up in the app and clash with my own classes (unless I use different table names in which case both original and my tables are created).
As for default, you should probably introduce default attribute, something like DbTypeDefault("expression") so default expressions would be divided from db type because current system does not allow configuring primary keys with default value because all foreign key will be defined with the same value. Separate default attribute could solve this issue.
Of course you can't predict all situations and combinations, no one expects that.
However, choosing random guids for primary keys and allowing database to create clustered index does not fall into that category. This is fundamental mistake, there is no one single explanation that could validate such choice.
Because there is no situation where this could work, except perhaps in very small databases (and you most certainly do not market XAF as solution only for very small projects with very little data over longer timeframe) or in specific apps which have very little data entry, but once you reach around million records in a table - which you will with time (which is still a VERY small database) things go exponentially bad.
For instance, we got "threat" mail today from a customer mostly affected by this. His 5 second action went from 5 minutes to 25 minutes in just a week. Because database engine has more and more work to reorder data as there are more and more pages which needs to be moved on EVERY single CRUD action.
And enabling audit log is a suicide, I can attest that with my decade shortened life span.
This decision is a time bomb in any application that will accumulate data over years and has daily data input and you really need to fix this.
I am not saying you should stop using guids, but you should either move to sequential guids using above posted method (preferred solution) or making sure no clustered indices are created on random generated guid key.
Otherwise people will always reach a point where application will start to degrade in CRUD performance rapidly and will face huge issue because they will have little time to solve the problem AND will have to solve it on live, production databases on multiple sites with very angry customers.
Regards,
Mario
I've read this thread with interest while searching for something else. We also moved over to sequential guid generation a few years ago, but have not done any guid 'renumbering' of existing data.
We have a customer with a fairly significant XAF based database - mdf size us 365GB! That represents about 10 years of data. There is quite a lot of empty space in that though - and scope to prune data. My local copy of the same DB post cleanup is 248GB.
We've rolled our own audit trail functionality - we had hoped to periodically purge this, but customers like to hold on to everything.
What we've found is effective is to periodically copy out stale audit data from the main database to a satellite audit database. The XAF UI app can search across both databases and aggregate results for audit viewing. This audit database is currently 124GB.
Somewhat surprisingly (to me anyway) this all still continues to work - despite the database sizes.
Attached is a small SQL Server script I've knocked up which can be used to create a script to introduce a new int clustered index.
Thanks for sharing, Chris. Would you please share your results before and after this change as well as clarify your database size? Thanks in advance.
Hi Dennis - I'll try to, but there are many variables in the mix.
I have some very superficial timings on a 3.5m row table with a mix of random guids and sequential guids. 5 inserts - times in mS.
Before: 1582, 1195, 475, 608, 413
After: 71, 1, 1, 1, 1.
A 6.4m row table, again with a mix of random guids and sequential guids. 5 inserts - times in mS.
Before: 110, 92, 92, 42, 64
After: 37, 2, 2, 2, 2.
I'm deploying changes to a handful of tables within a customer's UAT database to see if there are any negative impact on performance and to test under a diminished multi-user load. I'm not really a DBA, so there is a bit of experimentation going on here.
All of this is very much in exercise of evaluation. I think the database itself is about 400-450GB with a mix of tables which have random/sequential and sequential only GUID PKs. This is a XAF app which first went live in 2010 and has scaled remarkably well for our customer over the last 10 years.
Cheers
Chris
Thank you, Chris.
>>I think the database itself is about 400-450GB
WOW!
:D it's actually a little larger than that… our customers never want to delete/purge anything. So, we have monthly batch jobs which copy out older audit data to a separate (less critical) database - which adds another 210Gb. When the user wants to view audit information for an instance this is fetched across two DB connections. We have 170m + 356m rows in backed up audit and non backed audit tables respectively.
When we have some spare time we'll be coming up with some strategies to reduce the content of these - not all data needs auditing for the same amount of time.
>>copy out older audit data to a separate (less critical) database
Cool, we too.