{"action":"created","issue":{"url":"https://api.github.com/repos/aspnet/EntityFramework/issues/6666","repository_url":"https://api.github.com/repos/aspnet/EntityFramework","labels_url":"https://api.github.com/repos/aspnet/EntityFramework/issues/6666/labels{/name}","comments_url":"https://api.github.com/repos/aspnet/EntityFramework/issues/6666/comments","events_url":"https://api.github.com/repos/aspnet/EntityFramework/issues/6666/events","html_url":"https://github.com/aspnet/EntityFramework/issues/6666","id":180741153,"number":6666,"title":"SaveChanges() deadlocks in overlapping scenarios","user":{"login":"divega","id":1253414,"avatar_url":"https://avatars.githubusercontent.com/u/1253414?v=3","gravatar_id":"","url":"https://api.github.com/users/divega","html_url":"https://github.com/divega","followers_url":"https://api.github.com/users/divega/followers","following_url":"https://api.github.com/users/divega/following{/other_user}","gists_url":"https://api.github.com/users/divega/gists{/gist_id}","starred_url":"https://api.github.com/users/divega/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/divega/subscriptions","organizations_url":"https://api.github.com/users/divega/orgs","repos_url":"https://api.github.com/users/divega/repos","events_url":"https://api.github.com/users/divega/events{/privacy}","received_events_url":"https://api.github.com/users/divega/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/aspnet/EntityFramework/labels/closed-fixed","name":"closed-fixed","color":"777777"},{"url":"https://api.github.com/repos/aspnet/EntityFramework/labels/type-bug","name":"type-bug","color":"f7c6c7"}],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/aspnet/EntityFramework/milestones/19","html_url":"https://github.com/aspnet/EntityFramework/milestones/1.1.0-preview1","labels_url":"https://api.github.com/repos/aspnet/EntityFramework/milestones/19/labels","id":1748298,"number":19,"title":"1.1.0-preview1","description":"","creator":{"login":"rowanmiller","id":4741851,"avatar_url":"https://avatars.githubusercontent.com/u/4741851?v=3","gravatar_id":"","url":"https://api.github.com/users/rowanmiller","html_url":"https://github.com/rowanmiller","followers_url":"https://api.github.com/users/rowanmiller/followers","following_url":"https://api.github.com/users/rowanmiller/following{/other_user}","gists_url":"https://api.github.com/users/rowanmiller/gists{/gist_id}","starred_url":"https://api.github.com/users/rowanmiller/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rowanmiller/subscriptions","organizations_url":"https://api.github.com/users/rowanmiller/orgs","repos_url":"https://api.github.com/users/rowanmiller/repos","events_url":"https://api.github.com/users/rowanmiller/events{/privacy}","received_events_url":"https://api.github.com/users/rowanmiller/received_events","type":"User","site_admin":false},"open_issues":20,"closed_issues":227,"state":"open","created_at":"2016-05-04T18:23:47Z","updated_at":"2016-10-06T23:05:12Z","due_on":null,"closed_at":null},"comments":4,"created_at":"2016-10-03T20:40:29Z","updated_at":"2016-10-06T23:33:21Z","closed_at":"2016-10-06T23:05:12Z","body":"### Steps to reproduce\r\n\r\nThe original repro steps were provided by @nathana1 and apply directly to PostgreSQL. Later it was confirmed that the issue affects SQL Server when transaction are being used (the default behavior for SaveChanges()) as well and should be reproducible on SQL Server with a subset of these steps:\r\n\r\n1. Git clone https://github.com/aspnet/benchmarks.git\r\n2. Run build.ps1 to setup everything up\r\n3. Setup a client as directed here:  https://github.com/aspnet/benchmarks#generating-load\r\n4. Modify https://github.com/aspnet/benchmarks/blob/dev/src/Benchmarks/appsettings.json with connection string to your db and add this field:  \"Database\": \"postgresql\"\r\n5. Modify either https://github.com/aspnet/benchmarks/blob/dev/src/Benchmarks/Data/ApplicationDbContext.cs#L28 to enable batching or https://github.com/aspnet/benchmarks/blob/dev/src/Benchmarks/Data/RawDb.cs#L106 to disable sorting (depending on if you want to look at raw or ef)\r\n6. Make sure the connection string includes Maximum Pool Size=1024 and that there can be enough connections, e.g. posgresql.conf can contain max_connections = 2000.\r\n7. On the web server run:  dotnet run urls=http://*:5000 scenarios=update\r\n - The first time it runs it will populate the database so it might take a little while\r\n8. On the client machine run: wrk -c 512 -d 10 -t 16 http://10.0.1.20:5000/updates/raw?queries=20\r\n - Substitute ef for raw in url to switch\r\n\r\n\r\n### The issue\r\nThe test shows initially a spike of throughput but then quickly the throughput goes down near zero, with most of the requests failing. Closer examination shows that the reason for the failure is deadlocks.\r\n\r\nAs we understand them is that the test will often generate two or more requests that try to make changes to two or more of the same rows, but the changes are performed in arbitrary order therefore the deadlocks. \r\n\r\nPreviously in 1.1 we added a feature to EF to allow disabling transactions during `SaveChanges()` (see https://github.com/aspnet/EntityFramework/issues/6339) mainly because we observed that this helped with these tests perform much better. However the reason that helped was that each individual statement in the batches generated by EF Core got their own transactions, meaning that locks did not remain held for the whole batch. In this case ordering did not matter.\r\n\r\n### Proposed solution\r\nThrough the raw tests we have found that sorting the SQL statements (https://github.com/aspnet/benchmarks/blob/dev/src/Benchmarks/Data/RawDb.cs#L106) based on primary key values can help achieve a similar performance boost.\r\n\r\nThe proposal is to extend the topological sort performed by EF Core over individual operations to include an order over the primary key values. Otherwise use this order in MERGE operations for SQL Server.\r\n\r\nOnce we do this we should consider pulling the feature that allows opting out of transactions in `SaveChanges()` (see https://github.com/aspnet/EntityFramework/issues/6339), as there aren't any clear benefits and it adds some complexity to the API."},"comment":{"url":"https://api.github.com/repos/aspnet/EntityFramework/issues/comments/252117180","html_url":"https://github.com/aspnet/EntityFramework/issues/6666#issuecomment-252117180","issue_url":"https://api.github.com/repos/aspnet/EntityFramework/issues/6666","id":252117180,"user":{"login":"AndriySvyryd","id":6539701,"avatar_url":"https://avatars.githubusercontent.com/u/6539701?v=3","gravatar_id":"","url":"https://api.github.com/users/AndriySvyryd","html_url":"https://github.com/AndriySvyryd","followers_url":"https://api.github.com/users/AndriySvyryd/followers","following_url":"https://api.github.com/users/AndriySvyryd/following{/other_user}","gists_url":"https://api.github.com/users/AndriySvyryd/gists{/gist_id}","starred_url":"https://api.github.com/users/AndriySvyryd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AndriySvyryd/subscriptions","organizations_url":"https://api.github.com/users/AndriySvyryd/orgs","repos_url":"https://api.github.com/users/AndriySvyryd/repos","events_url":"https://api.github.com/users/AndriySvyryd/events{/privacy}","received_events_url":"https://api.github.com/users/AndriySvyryd/received_events","type":"User","site_admin":false},"created_at":"2016-10-06T23:33:21Z","updated_at":"2016-10-06T23:33:21Z","body":"@divega Yes, but Coherence is broken, so it might be a couple of days till we get a build in the feed."}}
{"actor":{"display_login":"AndriySvyryd"}}