Skip to content

Commit fcb3979

Browse files
haludihaludi
committed
RavenDB-25627 Avoid reading Blittalbe object concurrently
1 parent aca5fe7 commit fcb3979

10 files changed

Lines changed: 162 additions & 67 deletions

File tree

‎src/Raven.Server/Documents/Indexes/Static/CurrentIndexingScope.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ private bool ValidateSchema(BlittableJsonReaderObject doc, ErrorBuilder errorBui
300300
if (validators == null)
301301
{
302302
var schemaValidatorCache = QueryContext.Documents.DocumentDatabase.SchemaValidatorCache;
303-
return schemaValidatorCache.Validate(collection, doc, errorBuilder);
303+
return schemaValidatorCache == null || schemaValidatorCache.Validate(collection, doc, errorBuilder);
304304
}
305305

306306
return validators.TryGet(collection, out var validator) == false

‎src/Raven.Server/Documents/SchemaValidation/SchemaValidationHelper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public static string GetPublicType(Type type)
9898
return Integer;
9999
if (type == typeof(LazyNumberValue) || type == typeof(decimal))
100100
return Number;
101-
if (type == typeof(LazyStringValue) || type == typeof(LazyCompressedStringValue))
101+
if (type == typeof(LazyStringValue) || type == typeof(LazyCompressedStringValue) || type == typeof(string))
102102
return String;
103103
if (type == typeof(bool))
104104
return Boolean;

‎src/Raven.Server/Documents/SchemaValidation/Validators/Untyped/ConstantSchemaRuleValidator.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ public class ConstantSchemaRuleValidator : FixedValueSchemaRuleValidator
1111
// ReSharper disable once ConvertToPrimaryConstructor
1212
public ConstantSchemaRuleValidator(object constantValue)
1313
{
14-
_constantValue = ConvertTypeForComparison(constantValue);
14+
_constantValue = ConvertTypeForComparison(constantValue, true);
1515
_constantValueForError = IsString(_constantValue) ? $"\"{_constantValue}\"" : _constantValue;
1616
}
1717

1818
public override bool Validate(object value, ErrorBuilder errorBuilder)
1919
{
20-
//The order here is extremely important since when comparing between blittable objects the function uses the first object context and _constantValue is used concurrently
21-
if (Equals(ConvertTypeForComparison(value), _constantValue))
20+
//The order here is mandatory since the validator 'value' is used concurrently and we CloneForConcurrentRead in case it is blittable
21+
if (SafeConcurrentEquals(schemaValue: _constantValue, documentValue: value))
2222
return true;
2323

2424
var quoteIfString = IsString(value) ? "\"" : "";
@@ -28,7 +28,7 @@ public override bool Validate(object value, ErrorBuilder errorBuilder)
2828

2929
protected override bool CheckTypeAndGetValue(object value, out object tValue)
3030
{
31-
tValue = ConvertTypeForComparison(value);
31+
tValue = ConvertTypeForComparison(value, false);
3232
return true;
3333
}
3434
}

‎src/Raven.Server/Documents/SchemaValidation/Validators/Untyped/EnumSchemaRuleValidator.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ public class EnumSchemaRuleValidator : FixedValueSchemaRuleValidator
1212
// ReSharper disable once ConvertToPrimaryConstructor
1313
public EnumSchemaRuleValidator(IEnumerable<object> enums)
1414
{
15-
_enums = enums.Select(ConvertTypeForComparison).ToArray();
15+
_enums = enums.Select(v => ConvertTypeForComparison(v, true)).ToArray();
1616
}
1717

1818
public override bool Validate(object value, ErrorBuilder errorBuilder)
1919
{
20-
//The order here is extremely important since when comparing between blittable objects the function uses the first object context and _constantValue is used concurrently
21-
if (_enums.Any(x => Equals(value, x)))
20+
//The order here is mandatory since the validator 'value' is used concurrently and we CloneForConcurrentRead in case it is blittable
21+
if (_enums.Any(x => SafeConcurrentEquals(schemaValue:x, documentValue:value)))
2222
return true;
2323

2424
var quoteIfString = IsString(value) ? "\"" : "";
@@ -28,7 +28,7 @@ public override bool Validate(object value, ErrorBuilder errorBuilder)
2828

2929
protected override bool CheckTypeAndGetValue(object value, out object tValue)
3030
{
31-
tValue = ConvertTypeForComparison(value);
31+
tValue = ConvertTypeForComparison(value, false);
3232
return true;
3333
}
3434
}

‎src/Raven.Server/Documents/SchemaValidation/Validators/Untyped/FixedValueSchemaRuleValidator.cs‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,40 @@ namespace Raven.Server.Documents.SchemaValidation.Validators.Untyped;
55

66
public abstract class FixedValueSchemaRuleValidator : SchemaRuleValidator<object>
77
{
8-
protected static object ConvertTypeForComparison(object v)
8+
protected static object ConvertTypeForComparison(object v, bool clone)
99
{
1010
return v switch
1111
{
1212
LazyNumberValue lnv => (decimal)lnv,
1313
long lv => (decimal)lv,
14-
decimal
15-
or LazyStringValue
16-
or LazyCompressedStringValue
17-
or BlittableJsonReaderObject
18-
or BlittableJsonReaderArray
19-
or bool
20-
or null => v,
14+
LazyStringValue or LazyCompressedStringValue => v.ToString(),
15+
BlittableJsonReaderObject obj => clone ? obj.CloneOnTheSameContext() : obj,
16+
BlittableJsonReaderArray array => clone ? array.Clone() : array,
17+
decimal or bool or null => v,
2118
_ => throw new InvalidOperationException($"The type {v.GetType()} is not supported.")
2219
};
2320
}
2421

22+
23+
protected static bool SafeConcurrentEquals(object schemaValue, object documentValue)
24+
{
25+
if (schemaValue is BlittableJsonReaderObject schemaObj)
26+
{
27+
if (documentValue is not BlittableJsonReaderObject documentObj)
28+
return false;
29+
return schemaObj.CloneForConcurrentRead(documentObj._context).Equals(documentObj);
30+
}
31+
32+
if (schemaValue is BlittableJsonReaderArray schemaArray)
33+
{
34+
if (documentValue is not BlittableJsonReaderArray documentArray)
35+
return false;
36+
return schemaArray.CloneForConcurrentRead(documentArray._context).Equals(documentArray);
37+
}
38+
39+
return Equals(schemaValue, documentValue);
40+
}
41+
2542
protected static bool IsString(object constantValue)
2643
{
2744
return SchemaValidationHelper.GetPublicTypeOfObj(constantValue) == SchemaValidationHelper.String;

‎src/Sparrow/Json/BlittableJsonReaderArray.cs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,26 @@ public BlittableJsonReaderArray Clone(JsonOperationContext context, BlittableJso
8787
? _parent.Clone(context)
8888
: context.ReadObject(new DynamicJsonValue { [RootArrayHolderPropertyName] = this }, "array holder", usageMode);
8989

90-
if(arrayHolder.TryGet(RootArrayHolderPropertyNameSegment, out BlittableJsonReaderArray array) == false)
90+
return GetRootArrayFromParent(arrayHolder);
91+
}
92+
93+
public BlittableJsonReaderArray CloneForConcurrentRead(JsonOperationContext externalContext)
94+
{
95+
// when we read a blittable we do also some allocations and use context's path cache (e.g. InsertionOrderProperties, ReadStringLazily, GetPropertyByIndex)
96+
// we cannot use internal BlittableJsonReaderBase._context for that purpose since it must not be used concurrently
97+
// we have to provide external context that will be used for that purpose during the read action
98+
// note that we don't copy the blittable data but still read from the original pointer
99+
AssertContextNotDisposed(externalContext);
100+
101+
var parent = _parent.CloneForConcurrentRead(externalContext);
102+
return GetRootArrayFromParent(parent);
103+
}
104+
105+
private static BlittableJsonReaderArray GetRootArrayFromParent(BlittableJsonReaderObject parent)
106+
{
107+
if (parent.TryGet(RootArrayHolderPropertyNameSegment, out BlittableJsonReaderArray array) == false)
91108
throw new InvalidOperationException("Couldn't find array");
92-
109+
93110
array.ArrayIsRoot();
94111
return array;
95112
}

‎src/Sparrow/Json/BlittableJsonReaderObject.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,8 +1122,10 @@ public BlittableJsonReaderObject Clone(JsonOperationContext context)
11221122
return context.ReadObject(this, null);
11231123
}
11241124

1125+
//https://issues.hibernatingrhinos.com/issue/RavenDB-25633/Implement-CloneForConcurrentRead-to-work-with-nested-objects
11251126
public BlittableJsonReaderObject CloneForConcurrentRead(JsonOperationContext externalContext)
11261127
{
1128+
Debug.Assert(HasParent == false);
11271129
// when we read a blittable we do also some allocations and use context's path cache (e.g. InsertionOrderProperties, ReadStringLazily, GetPropertyByIndex)
11281130
// we cannot use internal BlittableJsonReaderBase._context for that purpose since it must not be used concurrently
11291131
// we have to provide external context that will be used for that purpose during the read action

‎test/SlowTests/SchemaValidation/BasicIntegrationSchemaValidationTests.cs‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -606,52 +606,6 @@ await AssertWaitForTrueAsync(async () =>
606606
Assert.StartsWith("Raven.Client.Exceptions.SchemaValidation.SchemaValidationException: The value at 'Prop' must be '\"123\"', but it is '\"1234\"'.", e.Message);
607607
}
608608

609-
[RavenFact(RavenTestCategory.JavaScript)]
610-
public async Task SchemaValidation_WhenExternalReplicateInvalidData_ShouldNotThrow()
611-
{
612-
var schemaDefinitionObj = new DynamicJsonValue { [SVC.Properties] = new DynamicJsonValue { ["Prop"] = new DynamicJsonValue { [SVC.Const] = "123" } } };
613-
614-
using var context = JsonOperationContext.ShortTermSingleUse();
615-
using var schemaDefinition = context.ReadObject(schemaDefinitionObj, "test object");
616-
using var source = GetDocumentStore();
617-
618-
const string id = "random-id";
619-
using (var session = source.OpenAsyncSession())
620-
{
621-
await session.StoreAsync(new TestObj { Prop = "1234" }, id);
622-
await session.SaveChangesAsync();
623-
}
624-
625-
using var destination = GetDocumentStore();
626-
var configuration = new SchemaValidationConfiguration
627-
{
628-
Disabled = false,
629-
ValidatorsPerCollection = new Dictionary<string, SchemaDefinition>
630-
{
631-
{ "TestObjs", new SchemaDefinition { Schema = schemaDefinition.ToString() } }
632-
}
633-
};
634-
await destination.Maintenance.SendAsync(new ConfigureSchemaValidationOperation(configuration));
635-
636-
await SetupReplicationAsync(source, destination);
637-
638-
await AssertWaitForTrueAsync(async () =>
639-
{
640-
using var session = destination.OpenAsyncSession();
641-
var load = await session.LoadAsync<TestObj>(id);
642-
return load != null;
643-
});
644-
645-
var e = await Assert.ThrowsAnyAsync<RavenException>(async () =>
646-
{
647-
using var session = destination.OpenAsyncSession();
648-
var load = await session.LoadAsync<TestObj>(id);
649-
load.Prop2 = "something";
650-
await session.SaveChangesAsync();
651-
});
652-
Assert.StartsWith("Raven.Client.Exceptions.SchemaValidation.SchemaValidationException: The value at 'Prop' must be '\"123\"', but it is '\"1234\"'.", e.Message);
653-
}
654-
655609
[RavenFact(RavenTestCategory.JavaScript)]
656610
public async Task SchemaValidation_WhenDefineSchemaOnMetadata_ShouldReject()
657611
{

‎test/SlowTests/SchemaValidation/SchemaValidationSlowUnitTests.cs‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
using System.Linq;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
using FastTests.SchemaValidation;
5+
using Raven.Server.Documents.SchemaValidation;
6+
using Sparrow.Json;
7+
using Sparrow.Json.Parsing;
8+
using Tests.Infrastructure;
9+
using Xunit;
10+
using Xunit.Abstractions;
11+
using SVC = Raven.Server.Documents.SchemaValidation.SchemaValidatorConstants;
12+
13+
namespace SlowTests.SchemaValidation;
14+
15+
public class SchemaValidationSlowUnitTests : SchemaValidationTestsBase
16+
{
17+
public SchemaValidationSlowUnitTests(ITestOutputHelper output) : base(output)
18+
{
19+
}
20+
21+
[RavenFact(RavenTestCategory.JavaScript)]
22+
public async Task SchemaValidation_WhenValidateObjectConstantWithMassiveConcurrency()
23+
{
24+
const int concurrency = 100;
25+
26+
using var context = JsonOperationContext.ShortTermSingleUse();
27+
28+
var schemaValidator = new SchemaValidator();
29+
var jsonValue = new DynamicJsonValue();
30+
Fill(jsonValue, 5);
31+
32+
var schemaDefinition = new DynamicJsonValue
33+
{
34+
[SVC.Type] = "object",
35+
[SVC.Properties] = new DynamicJsonValue
36+
{
37+
["objectProp"] = new DynamicJsonValue { [SVC.Const] = jsonValue }
38+
}
39+
};
40+
using var _ = ReadObjectOnNewCtx(schemaDefinition, out var blitSchemaDefinition);
41+
schemaValidator.Init(blitSchemaDefinition);
42+
43+
using var barrier = new Barrier(concurrency);
44+
var tasks = Enumerable.Range(0, concurrency).Select(_ =>
45+
{
46+
var ctx = ReadObjectOnNewCtx(new DynamicJsonValue { ["objectProp"] = jsonValue }, out var obj);
47+
48+
return Task.Run(() =>
49+
{
50+
using var _ = ctx;
51+
barrier.SignalAndWait();
52+
Assert.True(schemaValidator.Validate(obj, out var errors), errors);
53+
});
54+
}).ToArray();
55+
await Task.WhenAll(tasks);
56+
}
57+
58+
[RavenFact(RavenTestCategory.JavaScript)]
59+
public async Task SchemaValidation_WhenValidateObjectEnumWithMassiveConcurrency()
60+
{
61+
const int concurrency = 100;
62+
63+
using var context = JsonOperationContext.ShortTermSingleUse();
64+
65+
var schemaValidator = new SchemaValidator();
66+
var jsonValue = new DynamicJsonValue();
67+
Fill(jsonValue, 5);
68+
69+
var schemaDefinition = new DynamicJsonValue
70+
{
71+
[SVC.Type] = "object",
72+
[SVC.Properties] = new DynamicJsonValue
73+
{
74+
["objectProp"] = new DynamicJsonValue { [SVC.Enum] = new DynamicJsonArray{jsonValue} }
75+
}
76+
};
77+
using var _ = ReadObjectOnNewCtx(schemaDefinition, out var blitSchemaDefinition);
78+
schemaValidator.Init(blitSchemaDefinition);
79+
80+
using var barrier = new Barrier(concurrency);
81+
var tasks = Enumerable.Range(0, concurrency).Select(_ =>
82+
{
83+
var ctx = ReadObjectOnNewCtx(new DynamicJsonValue { ["objectProp"] = jsonValue }, out var obj);
84+
85+
return Task.Run(() =>
86+
{
87+
using var _ = ctx;
88+
barrier.SignalAndWait();
89+
Assert.True(schemaValidator.Validate(obj, out var errors), errors);
90+
});
91+
}).ToArray();
92+
await Task.WhenAll(tasks);
93+
}
94+
95+
private static void Fill(DynamicJsonValue parent, int depth)
96+
{
97+
parent["strprop"] = "somevalue";
98+
99+
for (var i = 0; i < depth; i++)
100+
{
101+
var obj = new DynamicJsonValue();
102+
Fill(obj, depth-1);
103+
parent[$"prop{i}"] = obj;
104+
}
105+
}
106+
}

‎test/SlowTests/SlowTests.csproj‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@
161161
<ItemGroup>
162162
<Folder Include="Core\AdminConsole" />
163163
<Folder Include="SchemaUpgrade\Issues\VoronCurrentVersion\" />
164-
<Folder Include="SchemaValidation\" />
165164
<Folder Include="Server\Basic" />
166165
<Folder Include="Server\Documents\AI\Embeddings\QueryEmbeddingsBatchTest\" />
167166
<Folder Include="Server\Documents\Notifications" />

0 commit comments

Comments
 (0)