RavenDB-17793 : address PR comments - throw instead of debug asserts,…

… add more info to error messages
aviv86 · Feb 25, 2024 · 9e4068e · 9e4068e
1 parent 49e69a4
commit 9e4068e
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 14 deletions.
diff --git a/src/Raven.Server/ServerWide/Commands/Sharding/DeletePrefixedShardingSettingCommand.cs b/src/Raven.Server/ServerWide/Commands/Sharding/DeletePrefixedShardingSettingCommand.cs
@@ -1,6 +1,6 @@
-using System.Diagnostics;
-using Raven.Client.ServerWide;
+using Raven.Client.ServerWide;
 using Raven.Client.ServerWide.Sharding;
+using Raven.Server.Rachis;
 using Raven.Server.ServerWide.Sharding;
 using Raven.Server.Utils;  
 using Sparrow.Json.Parsing;
@@ -9,21 +9,22 @@ namespace Raven.Server.ServerWide.Commands.Sharding
 {
     public sealed class DeletePrefixedShardingSettingCommand : UpdateDatabaseCommand
     {
-        public PrefixedShardingSetting Prefix;
+        public PrefixedShardingSetting Setting;
 
         public DeletePrefixedShardingSettingCommand()
         {
         }
 
-        public DeletePrefixedShardingSettingCommand(PrefixedShardingSetting prefix, string database, string raftId) : base(database, raftId)
+        public DeletePrefixedShardingSettingCommand(PrefixedShardingSetting setting, string database, string raftId) : base(database, raftId)
         {
-            Prefix = prefix;
+            Setting = setting;
         }
 
         public override void UpdateDatabaseRecord(DatabaseRecord record, long etag)
         {
-            int prefixIndex = record.Sharding.Prefixed.BinarySearch(Prefix, PrefixedSettingComparer.Instance);
-            Debug.Assert(prefixIndex >= 0, $"prefix {Prefix.Prefix} doesn't exist");
+            int prefixIndex = record.Sharding.Prefixed.BinarySearch(Setting, PrefixedSettingComparer.Instance);
+            if (prefixIndex < 0)
+                throw new RachisApplyException($"Prefixed setting '{Setting.Prefix}' was not found in sharding configuration");
 
             var prefixRangeStart = record.Sharding.Prefixed[prefixIndex].BucketRangeStart;
             var nextPrefixRangeStart = prefixRangeStart + ShardHelper.NumberOfBuckets;
@@ -52,7 +53,7 @@ public override void UpdateDatabaseRecord(DatabaseRecord record, long etag)
 
         public override void FillJson(DynamicJsonValue json)
         {
-            json[nameof(Prefix)] = Prefix.ToJson();
+            json[nameof(Setting)] = Setting.ToJson();
         }
     }
 }
diff --git a/src/Raven.Server/ServerWide/Commands/Sharding/StartBucketMigrationCommand.cs b/src/Raven.Server/ServerWide/Commands/Sharding/StartBucketMigrationCommand.cs
@@ -119,19 +119,19 @@ private void ProcessSubscriptionsForMigration(ClusterOperationContext context, S
         private void AssertDestinationShardExists(ShardingConfiguration shardingConfiguration)
         {
             if (shardingConfiguration.Shards.ContainsKey(DestinationShard) == false)
-                throw new RachisApplyException($"Destination shard {DestinationShard} doesn't exists");
+                throw new RachisApplyException($"Database '{DatabaseName}' : Failed to start migration of bucket '{Bucket}'. Destination shard {DestinationShard} doesn't exist");
 
             if (string.IsNullOrEmpty(Prefix)) 
                 return;
 
             // prefixed bucket range
             var index = shardingConfiguration.Prefixed.BinarySearch(new PrefixedShardingSetting(Prefix), PrefixedSettingComparer.Instance);
             if (index < 0)
-                throw new RachisApplyException($"Prefix {Prefix} doesn't exists");
+                throw new RachisApplyException($"Database '{DatabaseName}' : Failed to start migration of bucket '{Bucket}'. Prefix {Prefix} doesn't exist");
 
             var shards = shardingConfiguration.Prefixed[index].Shards;
             if (shards == null || shards.Contains(DestinationShard) == false)
-                throw new RachisApplyException($"Destination shard {DestinationShard} doesn't exists");
+                throw new RachisApplyException($"Database '{DatabaseName}' : Failed to start migration of bucket '{Bucket}'. Destination shard {DestinationShard} doesn't exist");
         }
 
         public override void FillJson(DynamicJsonValue json)

diff --git a/src/Raven.Server/ServerWide/Commands/Sharding/UpdatePrefixedShardingSettingCommand.cs b/src/Raven.Server/ServerWide/Commands/Sharding/UpdatePrefixedShardingSettingCommand.cs
@@ -1,6 +1,6 @@
-using System.Diagnostics;
-using Raven.Client.ServerWide;
+using Raven.Client.ServerWide;
 using Raven.Client.ServerWide.Sharding;
+using Raven.Server.Rachis;
 using Raven.Server.ServerWide.Sharding;
 using Sparrow.Json.Parsing;
 
@@ -22,7 +22,8 @@ public UpdatePrefixedShardingSettingCommand(PrefixedShardingSetting setting, str
         public override void UpdateDatabaseRecord(DatabaseRecord record, long etag)
         {
             var location = record.Sharding.Prefixed.BinarySearch(Setting, PrefixedSettingComparer.Instance);
-            Debug.Assert(location >= 0, $"Prefixed setting '{Setting.Prefix}' was not found in sharding configuration");
+            if (location < 0)
+                throw new RachisApplyException($"Prefixed setting '{Setting.Prefix}' was not found in sharding configuration");
 
             var oldSetting = record.Sharding.Prefixed[location];
             oldSetting.Shards = Setting.Shards;