Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Making sure that deletes are properly atomic in multi threading scena…

…rios
  • Loading branch information...
commit 02e68bdb9786139220dbc58bafd04e30a46dca2d 1 parent fb58c99
@ayende ayende authored
View
83 Raven.Database/DocumentDatabase.cs
@@ -585,56 +585,59 @@ public bool Delete(string key, Guid? etag, TransactionInformation transactionInf
if (key == null) throw new ArgumentNullException("key");
key = key.Trim();
- var deleted = false;
- log.Debug("Delete a document with key: {0} and etag {1}", key, etag);
- RavenJObject metadataVar = null;
- TransactionalStorage.Batch(actions =>
+ lock(putSerialLock)
{
- if (transactionInformation == null)
+ var deleted = false;
+ log.Debug("Delete a document with key: {0} and etag {1}", key, etag);
+ RavenJObject metadataVar = null;
+ TransactionalStorage.Batch(actions =>
{
- AssertDeleteOperationNotVetoed(key, null);
-
- DeleteTriggers.Apply(trigger => trigger.OnDelete(key, null));
-
- if (actions.Documents.DeleteDocument(key, etag, out metadataVar))
+ if (transactionInformation == null)
{
- deleted = true;
- foreach (var indexName in IndexDefinitionStorage.IndexNames)
- {
- AbstractViewGenerator abstractViewGenerator = IndexDefinitionStorage.GetViewGenerator(indexName);
- if (abstractViewGenerator == null)
- continue;
+ AssertDeleteOperationNotVetoed(key, null);
- var token = metadataVar.Value<string>(Constants.RavenEntityName);
+ DeleteTriggers.Apply(trigger => trigger.OnDelete(key, null));
- if (token != null && // the document has a entity name
- abstractViewGenerator.ForEntityNames.Count > 0) // the index operations on specific entities
+ if (actions.Documents.DeleteDocument(key, etag, out metadataVar))
+ {
+ deleted = true;
+ foreach (var indexName in IndexDefinitionStorage.IndexNames)
{
- if (abstractViewGenerator.ForEntityNames.Contains(token) == false)
+ AbstractViewGenerator abstractViewGenerator = IndexDefinitionStorage.GetViewGenerator(indexName);
+ if (abstractViewGenerator == null)
continue;
- }
- string indexNameCopy = indexName;
- var task = actions.GetTask<RemoveFromIndexTask>(x => x.Index == indexNameCopy, new RemoveFromIndexTask
- {
- Index = indexNameCopy
- });
- task.Keys.Add(key);
+ var token = metadataVar.Value<string>(Constants.RavenEntityName);
+
+ if (token != null && // the document has a entity name
+ abstractViewGenerator.ForEntityNames.Count > 0) // the index operations on specific entities
+ {
+ if (abstractViewGenerator.ForEntityNames.Contains(token) == false)
+ continue;
+ }
+
+ string indexNameCopy = indexName;
+ var task = actions.GetTask<RemoveFromIndexTask>(x => x.Index == indexNameCopy, new RemoveFromIndexTask
+ {
+ Index = indexNameCopy
+ });
+ task.Keys.Add(key);
+ }
+ DeleteTriggers.Apply(trigger => trigger.AfterDelete(key, transactionInformation));
}
- DeleteTriggers.Apply(trigger => trigger.AfterDelete(key, transactionInformation));
}
- }
- else
- {
- deleted = actions.Transactions.DeleteDocumentInTransaction(transactionInformation, key, etag);
- }
- workContext.ShouldNotifyAboutWork(() => "DEL " + key);
- });
- TransactionalStorage
- .ExecuteImmediatelyOrRegisterForSyncronization(() => DeleteTriggers.Apply(trigger => trigger.AfterCommit(key)));
+ else
+ {
+ deleted = actions.Transactions.DeleteDocumentInTransaction(transactionInformation, key, etag);
+ }
+ workContext.ShouldNotifyAboutWork(() => "DEL " + key);
+ });
+ TransactionalStorage
+ .ExecuteImmediatelyOrRegisterForSyncronization(() => DeleteTriggers.Apply(trigger => trigger.AfterCommit(key)));
- metadata = metadataVar;
- return deleted;
+ metadata = metadataVar;
+ return deleted;
+ }
}
public bool HasTransaction(Guid txId)
@@ -1188,7 +1191,7 @@ public BatchResult[] Batch(IEnumerable<ICommandData> commands)
var commandDatas = commands.ToArray();
int retries = 128;
- var shouldLock = commandDatas.Any(x => x is PutCommandData || x is PatchCommandData);
+ var shouldLock = commandDatas.Any(x => x is PutCommandData || x is PatchCommandData || x is DeleteCommandData);
var shouldRetryIfGotConcurrencyError = commandDatas.All(x => x is PatchCommandData);
bool shouldRetry = false;
if (shouldLock)
View
9 Raven.Storage.Esent/StorageActions/Documents.cs
@@ -421,6 +421,15 @@ public bool DeleteDocumentInTransaction(TransactionInformation transactionInform
Api.MakeKey(session, Documents, key, Encoding.Unicode, MakeKeyGrbit.NewKey);
if (Api.TrySeek(session, Documents, SeekGrbit.SeekEQ) == false)
{
+ if(etag != null && etag.Value != Guid.Empty)
+ {
+ throw new ConcurrencyException("DELETE attempted on document '" + key +
+ "' using a non current etag")
+ {
+ ActualETag = Guid.Empty,
+ ExpectedETag = etag.Value
+ };
+ }
logger.Debug("Document with key '{0}' was not found, and considered deleted", key);
return false;
}
View
31 Raven.Storage.Managed/TransactionStorageActions.cs
@@ -45,8 +45,8 @@ public Guid AddDocumentInTransaction(string key, Guid? etag, RavenJObject data,
var muninKey = (RavenJObject) readResult.Key.CloneToken();
muninKey["txId"] = transactionInformation.Id.ToByteArray();
- if (storage.Documents.UpdateKey(readResult.Key) == false)
- throw new ConcurrencyException("PUT attempted on document '" + muninKey +
+ if (storage.Documents.UpdateKey(muninKey) == false)
+ throw new ConcurrencyException("PUT attempted on document '" + key +
"' that is currently being modified by another transaction");
}
else
@@ -107,21 +107,30 @@ public bool DeleteDocumentInTransaction(TransactionInformation transactionInform
var nonTxResult = storage.Documents.Read(new RavenJObject { { "key", key } });
if (nonTxResult == null)
{
+
+ if (etag != null && etag.Value != Guid.Empty)
+ {
+ throw new ConcurrencyException("DELETE attempted on document '" + key +
+ "' using a non current etag")
+ {
+ ActualETag = Guid.Empty,
+ ExpectedETag = etag.Value
+ };
+ }
return false;
}
+ StorageHelper.AssertNotModifiedByAnotherTransaction(storage, this, key, nonTxResult, transactionInformation);
+
+ var muninKey = (RavenJObject)nonTxResult.Key.CloneToken();
+ muninKey["txId"] = transactionInformation.Id.ToByteArray();
+ if (storage.Documents.UpdateKey(muninKey) == false)
+ throw new ConcurrencyException("DELETE attempted on document '" + key +
+ "' that is currently being modified by another transaction");
+
var readResult = storage.DocumentsModifiedByTransactions.Read(new RavenJObject { { "key", key } });
- StorageHelper.AssertNotModifiedByAnotherTransaction(storage, this, key, readResult, transactionInformation);
AssertValidEtag(key, nonTxResult, readResult, etag, "DELETE");
- if (readResult != null)
- {
- ((RavenJObject)readResult.Key)["txId"] = transactionInformation.Id.ToByteArray();
- if (storage.Documents.UpdateKey(readResult.Key) == false)
- throw new ConcurrencyException("DELETE attempted on document '" + key +
- "' that is currently being modified by another transaction");
- }
-
storage.Transactions.UpdateKey(new RavenJObject
{
{"txId", transactionInformation.Id.ToByteArray()},
View
166 Raven.Tests/Issues/RavenDB_551.cs
@@ -1,19 +1,108 @@
using System;
+using System.Threading;
+using System.Threading.Tasks;
+using System.Transactions;
using Raven.Abstractions.Data;
using Raven.Abstractions.Exceptions;
+using Raven.Client.Document;
using Raven.Json.Linq;
using Xunit;
+using TransactionInformation = Raven.Abstractions.Data.TransactionInformation;
namespace Raven.Tests.Issues
{
public class RavenDB_551 : RavenTest
{
[Fact]
- public void CanGetErrorOnOptimisticDeleteInTransaction()
+ public void ManyConcurrentDeleteForSameId()
{
- using(var store = NewDocumentStore())
+ using(GetNewServer())
+ using (var store = new DocumentStore
+ {
+ Url = "http://localhost:8079"
+ })
+ {
+ store.EnlistInDistributedTransactions = true;
+ store.ResourceManagerId = new Guid("5402132f-32b5-423e-8b3c-b6e27c5e00fa");
+ store.Initialize();
+
+ string id;
+ int concurrentExceptionsThrown = 0;
+ int concurrentDeleted = 0;
+
+ using (
+ var tx = new TransactionScope(TransactionScopeOption.RequiresNew,
+ new TransactionOptions
+ {
+ Timeout = TimeSpan.FromSeconds(30),
+ IsolationLevel = IsolationLevel.ReadCommitted
+ }))
+ {
+ using (var session = store.OpenSession())
+ {
+ var t1 = new MyData();
+ session.Store(t1);
+ session.SaveChanges();
+
+ id = t1.Id;
+ }
+
+ tx.Complete();
+ }
+
+ Thread.Sleep(2000); //Waiting for transaction to commit
+
+ Parallel.For(0, 10, new ParallelOptions { MaxDegreeOfParallelism = 10 }, i =>
+ {
+ try
+ {
+ using (
+ var tx = new TransactionScope(TransactionScopeOption.RequiresNew,
+ new TransactionOptions
+ {
+ Timeout = TimeSpan.FromSeconds(30),
+ IsolationLevel = IsolationLevel.ReadCommitted
+ }))
+ {
+ using (var session = store.OpenSession())
+ {
+ session.Advanced.UseOptimisticConcurrency = true;
+
+ var myData = session.Load<MyData>(id);
+
+ Thread.Sleep(1000);
+
+ session.Delete(myData);
+ session.SaveChanges();
+ }
+
+ tx.Complete();
+ }
+
+ Interlocked.Increment(ref concurrentDeleted);
+
+ }
+ catch (Exception)
+ {
+ Interlocked.Increment(ref concurrentExceptionsThrown);
+ }
+ });
+
+ Assert.Equal(1, concurrentDeleted);
+ Assert.Equal(9, concurrentExceptionsThrown);
+ }
+ }
+
+ public class MyData
+ {
+ public string Id { get; set; }
+ }
+ [Fact]
+ public void CanGetErrorOnOptimisticDeleteInTransaction()
+ {
+ using (var store = NewDocumentStore())
{
- using(var session = store.OpenSession())
+ using (var session = store.OpenSession())
{
session.Store(new Item());
session.SaveChanges();
@@ -22,7 +111,7 @@ public void CanGetErrorOnOptimisticDeleteInTransaction()
{
Id = Guid.NewGuid()
};
- Assert.Throws<ConcurrencyException>(() =>
+ Assert.Throws<ConcurrencyException>(() =>
store.DocumentDatabase.Delete("items/1", Guid.NewGuid(), tx));
}
}
@@ -47,6 +136,73 @@ public void CanGetErrorOnOptimisticDeleteInTransactionWhenModifiedInTransaction(
}
}
- public class Item{}
+
+ [Fact]
+ public void CanGetErrorOnOptimisticDeleteInTransactionWhenDeletedInTransaction()
+ {
+ using (var store = NewDocumentStore())
+ {
+ using (var session = store.OpenSession())
+ {
+ session.Store(new Item());
+ session.SaveChanges();
+ }
+ var tx = new TransactionInformation
+ {
+ Id = Guid.NewGuid()
+ };
+ store.DocumentDatabase.Delete("items/1", null, tx);
+ Assert.Throws<ConcurrencyException>(() =>
+ store.DocumentDatabase.Delete("items/1", Guid.NewGuid(), tx));
+ }
+ }
+
+ [Fact]
+ public void CanGetErrorOnOptimisticDeleteInTransactionWhenDeletedInTransactionUsingOldEtag()
+ {
+ using (var store = NewDocumentStore())
+ {
+ using (var session = store.OpenSession())
+ {
+ session.Store(new Item());
+ session.SaveChanges();
+ }
+ var jsonDocument = store.DocumentDatabase.Get("items/1", null);
+ store.DocumentDatabase.Delete("items/1", null, new TransactionInformation
+ {
+ Id = Guid.NewGuid(),
+ Timeout = TimeSpan.FromSeconds(2)
+ });
+ Assert.Throws<ConcurrencyException>(() =>
+ store.DocumentDatabase.Delete("items/1", jsonDocument.Etag, new TransactionInformation
+ {
+ Id = Guid.NewGuid()
+ }));
+ }
+ }
+
+ [Fact]
+ public void CanGetErrorOnOptimisticDeleteInTransactionWhenDeletedInAnotherTransaction()
+ {
+ using (var store = NewDocumentStore())
+ {
+ using (var session = store.OpenSession())
+ {
+ session.Store(new Item());
+ session.SaveChanges();
+ }
+ store.DocumentDatabase.Delete("items/1", null, new TransactionInformation
+ {
+ Id = Guid.NewGuid()
+ });
+ Assert.Throws<ConcurrencyException>(() =>
+ store.DocumentDatabase.Delete("items/1", Guid.NewGuid(), new TransactionInformation
+ {
+ Id = Guid.NewGuid()
+ }));
+ }
+ }
+
+ public class Item { }
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.