From 83a4d2efed2fead9dbfa5e837cb13387520e8f28 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 24 Jan 2024 19:11:00 +0800 Subject: [PATCH] Backend(,Tests): avoid inconsistency ex if no inconsistency This is the first part of a bugfix for issue 250 [1]. The exception was misleading because it was not really an inconsistecy but rather lacking availability. [1] https://github.com/nblockchain/geewallet/issues/250 --- src/GWallet.Backend.Tests/FaultTolerance.fs | 33 ++++++++++++ .../FaultTolerantParallelClient.fs | 50 +++++++++++++++---- 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/src/GWallet.Backend.Tests/FaultTolerance.fs b/src/GWallet.Backend.Tests/FaultTolerance.fs index f0ea87142..7e73f500e 100644 --- a/src/GWallet.Backend.Tests/FaultTolerance.fs +++ b/src/GWallet.Backend.Tests/FaultTolerance.fs @@ -546,6 +546,39 @@ type FaultTolerance() = |> ignore ) Assert.That(inconsistencyEx.Message, IsString.WhichContains "received: 4, consistent: 2, required: 3") + [] + member __.``should not throw inconsistency exception if received results is less than required``() = + let numberOfConsistentResponsesToBeConsideredSafe = 2u + let consistencyConfig = + SpecificNumberOfConsistentResponsesRequired numberOfConsistentResponsesToBeConsideredSafe |> Some + let settings = defaultSettingsForNoConsistencyNoParallelismAndNoRetries consistencyConfig + + let someResult = 1 + + let aJob1 = + async { return someResult } + let aJob2 = + async { return raise SomeSpecificException } + let aJob3 = + async { return raise SomeSpecificException } + + let func1,func2,func3 = serverWithNoHistoryInfoBecauseIrrelevantToThisTest "aJob1" aJob1, + serverWithNoHistoryInfoBecauseIrrelevantToThisTest "aJob2" aJob2, + serverWithNoHistoryInfoBecauseIrrelevantToThisTest "aJob3" aJob3 + + let client = FaultTolerantParallelClient + dummy_func_to_not_save_server_because_it_is_irrelevant_for_this_test + + let notEngoughAvailableEx = + Assert.Throws(fun _ -> + client.Query + settings + [ func1; func2; func3 ] + |> Async.RunSynchronously + |> ignore + ) + Assert.That(notEngoughAvailableEx.Message, IsString.WhichContains "received: 1, unavailable: 2, required: 2") + [] member __.``using an average func instead of consistency``() = let job1 = diff --git a/src/GWallet.Backend/FaultTolerantParallelClient.fs b/src/GWallet.Backend/FaultTolerantParallelClient.fs index 295ea3cbd..fe32e907d 100644 --- a/src/GWallet.Backend/FaultTolerantParallelClient.fs +++ b/src/GWallet.Backend/FaultTolerantParallelClient.fs @@ -10,22 +10,40 @@ open Fsdk open GWallet.Backend.FSharpUtil.UwpHacks -type ResourcesUnavailabilityException (message: string, innerOrLastException: Exception) = - inherit Exception (message, innerOrLastException) +type ResourcesUnavailabilityException = + inherit Exception + + new(message: string, innerException: Exception) = { inherit Exception(message, innerException) } + new(message: string) = { inherit Exception(message) } type private TaskUnavailabilityException (message: string, innerException: Exception) = inherit ResourcesUnavailabilityException (message, innerException) -type private ServersUnavailabilityException (message: string, lastException: Exception) = - inherit ResourcesUnavailabilityException (message, lastException) +type ServersUnavailabilityException = + inherit ResourcesUnavailabilityException + + new(message: string, innerException: Exception) = { inherit ResourcesUnavailabilityException(message, innerException) } + new(message: string) = { inherit ResourcesUnavailabilityException(message) } type private NoneAvailableException (message:string, lastException: Exception) = inherit ServersUnavailabilityException (message, lastException) -type private NotEnoughAvailableException (message:string, lastException: Exception) = - inherit ServersUnavailabilityException (message, lastException) +type NotEnoughAvailableException = + inherit ServersUnavailabilityException + + new (message: string, innerException: Exception) = + { inherit ServersUnavailabilityException (message, innerException) } + new (totalNumberOfSuccesfulResultsObtained: uint32, + numberOfServersUnavailable: uint32, + numberOfConsistentResultsRequired: uint32) = + { inherit ServersUnavailabilityException ("Results obtained were not enough to be considered consistent" + + SPrintF3 " (received: %i, unavailable: %i, required: %i)" + totalNumberOfSuccesfulResultsObtained + numberOfServersUnavailable + numberOfConsistentResultsRequired) + } -type ResultInconsistencyException (totalNumberOfSuccesfulResultsObtained: int, +type ResultInconsistencyException (totalNumberOfSuccesfulResultsObtained: uint32, maxNumberOfConsistentResultsObtained: int, numberOfConsistentResultsRequired: uint32) = inherit Exception ("Results obtained were not enough to be considered consistent" + @@ -591,7 +609,8 @@ type FaultTolerantParallelClient<'K,'E when 'K: equality and 'K :> ICommunicatio retriesForInconsistency cancellationSource else - let totalNumberOfSuccesfulResultsObtained = executedServers.SuccessfulResults.Length + let totalNumberOfSuccesfulResultsObtained = uint executedServers.SuccessfulResults.Length + let totalNumberOfUnavailableServers = uint failedFuncs.Length // HACK: we do this as a quick fix wrt new OneServerConsistentWithCertainValueOrTwoServers setting, but we should // (TODO) rather throw a specific overload of ResultInconsistencyException about this mode being used @@ -609,9 +628,18 @@ type FaultTolerantParallelClient<'K,'E when 'K: equality and 'K :> ICommunicatio return failwith "resultsSoFar.Length != 0 but MeasureConsistency returns None, please report this bug" | (_,maxNumberOfConsistentResultsObtained)::_ -> if (retriesForInconsistency = settings.NumberOfRetriesForInconsistency) then - return raise (ResultInconsistencyException(totalNumberOfSuccesfulResultsObtained, - maxNumberOfConsistentResultsObtained, - numberOfConsistentResponsesRequired)) + if totalNumberOfSuccesfulResultsObtained >= numberOfConsistentResponsesRequired then + return raise (ResultInconsistencyException(totalNumberOfSuccesfulResultsObtained, + maxNumberOfConsistentResultsObtained, + numberOfConsistentResponsesRequired)) + else + return + raise + <| NotEnoughAvailableException( + totalNumberOfSuccesfulResultsObtained, + totalNumberOfUnavailableServers, + numberOfConsistentResponsesRequired + ) else return! QueryInternalImplementation settings