diff --git a/src/Raven.Server/Documents/CountersStorage.cs b/src/Raven.Server/Documents/CountersStorage.cs index 5f2c6e6a70b5..98f0afa7be2a 100644 --- a/src/Raven.Server/Documents/CountersStorage.cs +++ b/src/Raven.Server/Documents/CountersStorage.cs @@ -390,7 +390,12 @@ public string PutOrIncrementCounter(DocumentsOperationContext context, string do var lowerName = Encodings.Utf8.GetString(counterName.Content.Ptr, counterName.Content.Length); Slice countersGroupKey; - if (table.SeekOneBackwardByPrimaryKeyPrefix(documentKeyPrefix, counterKeySlice, out var existing)) + if (table.SeekOneBackwardByPrimaryKeyPrefix(documentKeyPrefix, counterKeySlice, out var existing) == false) + { + data = WriteNewCountersDocument(context, originalName: name, lowerName, value); + countersGroupKey = documentKeyPrefix; + } + else { countersGroupKeyScope = Slice.From(context.Allocator, existing.Read((int)CountersTable.CounterKey, out var size), size, out countersGroupKey); @@ -412,6 +417,8 @@ public string PutOrIncrementCounter(DocumentsOperationContext context, string do if (data.TryGet(CounterNames, out BlittableJsonReaderObject originalNames) == false) ThrowMissingProperty(counterKeySlice, CounterNames); + CheckForCorruptedCountersData(documentId, counters, originalNames); + var counterEtag = _documentsStorage.GenerateNextEtag(); counters.TryGetMember(lowerName, out object existingCounter); @@ -434,7 +441,8 @@ public string PutOrIncrementCounter(DocumentsOperationContext context, string do var existingChangeVector = TableValueToChangeVector(context, (int)CountersTable.ChangeVector, ref existing); using (data) { - SplitCounterGroup(context, collectionName, table, documentKeyPrefix, countersGroupKey, counters, dbIds, originalNames, existingChangeVector); + SplitCounterGroup(context, collectionName, table, documentKeyPrefix, countersGroupKey, counters, dbIds, originalNames, + existingChangeVector); } // now we retry and know that we have enough space @@ -446,10 +454,7 @@ public string PutOrIncrementCounter(DocumentsOperationContext context, string do if (existingCounter == null) { // new counter - originalNames.Modifications = new DynamicJsonValue - { - [lowerName] = name - }; + originalNames.Modifications = new DynamicJsonValue { [lowerName] = name }; } } @@ -461,11 +466,6 @@ public string PutOrIncrementCounter(DocumentsOperationContext context, string do } } } - else - { - data = WriteNewCountersDocument(context, originalName: name, lowerName, value); - countersGroupKey = documentKeyPrefix; - } var groupEtag = _documentsStorage.GenerateNextEtag(); var changeVector = _documentsStorage.GetNewChangeVector(context, groupEtag); @@ -520,6 +520,23 @@ public string PutOrIncrementCounter(DocumentsOperationContext context, string do } } + private static void CheckForCorruptedCountersData(string documentId, BlittableJsonReaderObject counterValues, BlittableJsonReaderObject counterNames) + { + // RavenDB-22835 + // counters data might be corrupted and needs to be fixed before adding/incrementing new counters + + var msg = $"Counters data of document '{documentId}' is corrupted. " + + $"Cannot add/increment counters while counter data is in this corrupted state. Please use FixCounters tool (/databases/*/counters/fix-document) in order to add/increment counters of this document"; + + if (counterValues.Count != counterNames.Count) + throw new InvalidDataException(msg); + + var counterValuesPropertyNames = counterValues.GetSortedPropertyNames(); + var counterNamesPropertyNames = counterNames.GetSortedPropertyNames(); + if (counterValuesPropertyNames.SequenceEqual(counterNamesPropertyNames) == false) + throw new InvalidDataException(msg); + } + internal static void ThrowMissingProperty(Slice counterKeySlice, string property) { throw new InvalidDataException($"Counter-Group document '{counterKeySlice}' is missing '{property}' property. Shouldn't happen"); @@ -1162,6 +1179,17 @@ public bool PutCounters(DocumentsOperationContext context, string documentId, st } } + public int FixCountersForDocuments(DocumentsOperationContext context, List docIds) + { + var numOfCounterGroupsFixed = 0; + foreach (var docId in docIds) + { + numOfCounterGroupsFixed += FixCountersForDocument(context, docId); + } + + return numOfCounterGroupsFixed; + } + public int FixCountersForDocument(DocumentsOperationContext context, string documentId) { List allNames = null; @@ -1188,8 +1216,6 @@ public int FixCountersForDocument(DocumentsOperationContext context, string docu data.TryGet(Values, out BlittableJsonReaderObject counterValues); data.TryGet(CounterNames, out BlittableJsonReaderObject counterNames); - if (counterValues == null || counterNames == null) - throw new InvalidDataException("Invalid CounterGroup data - missing counter values or counter names"); if (counterValues.Count == counterNames.Count) { @@ -1202,7 +1228,7 @@ public int FixCountersForDocument(DocumentsOperationContext context, string docu if (collection == null) { collection = TableValueToId(context, (int)CountersTable.Collection, ref tvr); - collectionName = _documentsStorage.ExtractCollectionName(context, collection); //todo + collectionName = _documentsStorage.ExtractCollectionName(context, collection); writeTable = GetOrCreateTable(context.Transaction.InnerTransaction, CountersSchema, collectionName, CollectionTableType.CounterGroups); } diff --git a/src/Raven.Server/Documents/Handlers/CountersHandler.cs b/src/Raven.Server/Documents/Handlers/CountersHandler.cs index 00e090957c16..894a1f75cfbe 100644 --- a/src/Raven.Server/Documents/Handlers/CountersHandler.cs +++ b/src/Raven.Server/Documents/Handlers/CountersHandler.cs @@ -27,6 +27,7 @@ using Sparrow.Json.Parsing; using Sparrow.Server; using Voron; +using System.Linq; namespace Raven.Server.Documents.Handlers { @@ -762,18 +763,23 @@ private static void ThrowMissingDocument(string docId) public class ExecuteFixCounterGroupsCommand : TransactionOperationsMerger.MergedTransactionCommand { - private readonly string _docId; + private readonly List _docIds; private readonly DocumentDatabase _database; - public ExecuteFixCounterGroupsCommand(DocumentDatabase database, string docId) + + public ExecuteFixCounterGroupsCommand(DocumentDatabase database, string docId) : this(database, [docId]) + { + } + + public ExecuteFixCounterGroupsCommand(DocumentDatabase database, List docIdsToFix) { - _docId = docId; + _docIds = docIdsToFix; _database = database; } protected override long ExecuteCmd(DocumentsOperationContext context) { - var numOfCounterGroupFixed = _database.DocumentsStorage.CountersStorage.FixCountersForDocument(context, _docId); + var numOfCounterGroupFixed = _database.DocumentsStorage.CountersStorage.FixCountersForDocuments(context, _docIds); return numOfCounterGroupFixed; } @@ -783,16 +789,27 @@ protected override long ExecuteCmd(DocumentsOperationContext context) } } + [RavenAction("/databases/*/counters/fix-document", "PATCH", AuthorizationStatus.ValidUser, EndpointType.Write)] + public async Task FixCounterGroupsForDocument() + { + var docId = GetStringQueryString("id"); + await FixCounterGroupsInternal(docId); + } + [RavenAction("/databases/*/counters/fix", "PATCH", AuthorizationStatus.ValidUser, EndpointType.Write)] - public async Task FixCounterGroups() + public async Task FixCounterGroupsForDatabase() => await FixCounterGroupsInternal(); + + private async Task FixCounterGroupsInternal(string id = null) { var first = GetBoolValueQueryString("first", required: false) ?? true; - var task = FixAllCounterGroups(); + + Task task = id == null + ? FixAllCounterGroups() + : Database.TxMerger.Enqueue(new ExecuteFixCounterGroupsCommand(Database, id)); if (first == false) { await task; - await NoContent(); return; } @@ -812,12 +829,13 @@ public async Task FixCounterGroups() toDispose.Add(requestExecutor); var cmd = new FixCounterGroupsCommand(Database.Name); - tasks.Add(requestExecutor.ExecuteAsync(cmd, context)); + + toDispose.Add(requestExecutor.ContextPool.AllocateOperationContext(out JsonOperationContext ctx)); + tasks.Add(requestExecutor.ExecuteAsync(cmd, ctx)); } } await Task.WhenAll(tasks); - await NoContent(); } finally { @@ -828,24 +846,64 @@ public async Task FixCounterGroups() } } + private async Task FixAllCounterGroups() { - using (Database.DocumentsStorage.ContextPool.AllocateOperationContext(out DocumentsOperationContext context)) - using (context.OpenReadTransaction()) + const int maxNumberOfDocsToFixInSingleTx = 1024; + int skip = 0; + string currentId = null; + bool corruptedDoc = false; + + while (true) { - var table = new Table(CountersStorage.CountersSchema, context.Transaction.InnerTransaction); - string currentId = null; + List docIdsToFix = new(); - foreach (var (_, tvh) in table.SeekByPrimaryKeyPrefix(Slices.BeforeAllKeys, Slices.Empty, skip: 0)) + using (Database.DocumentsStorage.ContextPool.AllocateOperationContext(out DocumentsOperationContext context)) + using (context.OpenReadTransaction()) { - using (var docId = CountersStorage.ExtractDocId(context, ref tvh.Reader)) + var table = new Table(CountersStorage.CountersSchema, context.Transaction.InnerTransaction); + + foreach (var (_, tvh) in table.SeekByPrimaryKeyPrefix(Slices.BeforeAllKeys, Slices.Empty, skip: skip)) { - if (docId == currentId) - continue; + skip++; + + using (var docId = CountersStorage.ExtractDocId(context, ref tvh.Reader)) + { + if (docId != currentId) + { + currentId = docId; + corruptedDoc = false; + } + + else if (corruptedDoc) + continue; + + using (var data = CountersStorage.GetCounterValuesData(context, ref tvh.Reader)) + { + data.TryGet(CountersStorage.Values, out BlittableJsonReaderObject counterValues); + data.TryGet(CountersStorage.CounterNames, out BlittableJsonReaderObject counterNames); + + if (counterValues.Count == counterNames.Count) + { + var counterValuesPropertyNames = counterValues.GetSortedPropertyNames(); + var counterNamesPropertyNames = counterNames.GetSortedPropertyNames(); + if (counterValuesPropertyNames.SequenceEqual(counterNamesPropertyNames)) + continue; + } + } + + corruptedDoc = true; + docIdsToFix.Add(docId); + } - currentId = docId; - await Database.TxMerger.Enqueue(new ExecuteFixCounterGroupsCommand(Database, docId)); + if (docIdsToFix.Count == maxNumberOfDocsToFixInSingleTx) + break; } + + if (docIdsToFix.Count == 0) + return; + + await Database.TxMerger.Enqueue(new ExecuteFixCounterGroupsCommand(Database, docIdsToFix)); } } } diff --git a/test/SlowTests/Issues/RavenDB_22835.cs b/test/SlowTests/Issues/RavenDB_22835.cs index 081a211fa8f4..a086c911e99e 100644 --- a/test/SlowTests/Issues/RavenDB_22835.cs +++ b/test/SlowTests/Issues/RavenDB_22835.cs @@ -6,6 +6,7 @@ using Raven.Client.Documents; using Raven.Client.Documents.Conventions; using Raven.Client.Documents.Smuggler; +using Raven.Client.Exceptions; using Raven.Client.Http; using Raven.Server.Documents; using Raven.Server.Documents.Replication.ReplicationItems; @@ -30,7 +31,7 @@ public RavenDB_22835(ITestOutputHelper output) : base(output) { } - [Fact] + [RavenFact(RavenTestCategory.Counters)] public async Task SplittingCountersManyTimes_ShouldNotCauseErrorsDuringImport() { var rand = new Random(12345); @@ -86,7 +87,7 @@ public async Task SplittingCountersManyTimes_ShouldNotCauseErrorsDuringImport() // all good - no exception was thrown during imports or when adding new counters } - [Fact] + [RavenFact(RavenTestCategory.Counters)] public async Task FixCorruptedCounterData() { var rand = new Random(12345); @@ -193,7 +194,7 @@ public async Task FixCorruptedCounterData() } } - [Fact] + [RavenFact(RavenTestCategory.Counters)] public async Task FixCorruptedCounterData_InCluster() { var rand = new Random(12345); @@ -292,7 +293,9 @@ public async Task FixCorruptedCounterData_InCluster() // call FixCounters endpoint using var requestExecutor = leaderStore.GetRequestExecutor(); - var cmd = new FixCountersCommand(); + + // use the single-doc endpoint + var cmd = new FixCountersCommand(id); using (cluster.Leader.ServerStore.ContextPool.AllocateOperationContext(out JsonOperationContext ctx)) await requestExecutor.ExecuteAsync(cmd, ctx); @@ -328,7 +331,7 @@ public async Task FixCorruptedCounterData_InCluster() } } - [Fact] + [RavenFact(RavenTestCategory.Counters)] public async Task FixCorruptedCounterData_MultipleDocs() { var rand = new Random(12345); @@ -393,8 +396,6 @@ public async Task FixCorruptedCounterData_MultipleDocs() Assert.False(counterNames.TryGet(firstCounter, out string _)); } - - AddMoreCountersForDoc(store, rand, id, numOfIterations: 10); } // call FixCounters endpoint @@ -431,6 +432,64 @@ public async Task FixCorruptedCounterData_MultipleDocs() } } + [RavenFact(RavenTestCategory.Counters)] + public async Task ShouldThrowOnAttemptToAddNewCounterIfCountersDataIsCorrupted() + { + var rand = new Random(12345); + const string id = "users/1"; + + using var store = GetDocumentStore(); + + using (var session = store.OpenSession()) + { + session.Store(new User(), id); + session.SaveChanges(); + } + + using (var session = store.OpenSession()) + { + var countersFor = session.CountersFor(id); + + for (int j = 0; j < 10; j++) + { + var strSize = (int)rand.NextInt64(5, 15); + var s = GenRandomString(rand, strSize); + + countersFor.Increment(s, j); + } + + session.SaveChanges(); + } + + var db = await GetDatabase(store.Database); + + // deliberately corrupt counters group data + CorruptCountersData(db); + + using (var session = store.OpenSession()) + { + session.CountersFor(id).Increment("new-counter", 123); + Assert.Throws(() => session.SaveChanges()); + } + + // call FixCounters tool + using (db.DocumentsStorage.ContextPool.AllocateOperationContext(out DocumentsOperationContext context)) + using (var tx = context.OpenWriteTransaction()) + { + var numOfFixes = db.DocumentsStorage.CountersStorage.FixCountersForDocument(context, id); + Assert.Equal(1, numOfFixes); + + tx.Commit(); + } + + // we should be able to add new counters now + using (var session = store.OpenSession()) + { + session.CountersFor(id).Increment("new-counter", 123); + session.SaveChanges(); + } + } + private unsafe void CorruptCountersData(DocumentDatabase database, string id = null) { using (database.DocumentsStorage.ContextPool.AllocateOperationContext(out DocumentsOperationContext context)) @@ -451,7 +510,6 @@ private unsafe void CorruptCountersData(DocumentDatabase database, string id = n } } - BlittableJsonReaderObject.PropertyDetails prop = default; BlittableJsonReaderObject data; using (data = GetCounterValuesData(context, ref tvr)) { @@ -531,9 +589,18 @@ private static void AddMoreCountersForDoc(IDocumentStore store, Random rand, str private class FixCountersCommand : RavenCommand { + private readonly string _docId; + + public FixCountersCommand(string docId = null) + { + _docId = docId; + } + public override HttpRequestMessage CreateRequest(JsonOperationContext ctx, ServerNode node, out string url) { - url = $"{node.Url}/databases/{node.Database}/counters/fix"; + url = _docId != null + ? $"{node.Url}/databases/{node.Database}/counters/fix-document?id={_docId}" + : $"{node.Url}/databases/{node.Database}/counters/fix"; return new HttpRequestMessage {