SonicRetry combined PR[CMSSW_15_1_0_pre6] #24
Conversation
…r method in TritonClient. Update BuildFile.xml and fix formatting in header files.
…tructor for TritonClient, and update BuildFile.xml to include Catch2 for testing.
…tests; remove old cfg
…lection; remove unused parameters and improve documentation.
| if (client_) { | ||
| client_->evaluate(); | ||
| } else { | ||
| edm::LogError("RetryActionBase") << "Client pointer is null, cannot evaluate."; |
There was a problem hiding this comment.
This should be an exception rather than a LogError. (It may actually need to be a return false or similar, because the call chain is client->finish() -> action->retry() -> action->eval(), and only finish() should actually emit an exception.)
There was a problem hiding this comment.
This comment has not been addressed yet
There was a problem hiding this comment.
It should be just return, similar to what client->evaluate does, if we want only finish() to throw exception
There was a problem hiding this comment.
But we need to let finish() know that the eval() failed somehow... I guess it could directly call finish(false) as is done in evaluate()/handle_exception(). we may want to rethink this pattern in general, though, as I am becoming concerned that it will lead to excessive stack depth.
| } catch (std::exception& e) { | ||
| edm::LogError("RetryActionDiffServer") << "Failed to retry with alternative server: " << e.what(); | ||
| } catch (...) { | ||
| edm::LogError("RetryActionDiffServe: rUnknownFailure") << "An unknown exception was thrown"; |
| @@ -0,0 +1,22 @@ | |||
| #!/bin/bash | |||
| if (client_) { | ||
| client_->evaluate(); | ||
| } else { | ||
| edm::LogError("RetryActionBase") << "Client pointer is null, cannot evaluate."; |
There was a problem hiding this comment.
This comment has not been addressed yet
Rebased #23 to CMSSW_15_1_0_pre6.
PR description:
RetryActionDiffServerfor SonicTriton usingTritonService’s server registry; remove per-action alternate server parameters.TritonClient::updateServer(TritonService::Server::fallbackName)to switch servers on retry, per review guidance.HeterogeneousCore/SonicTriton/test/test_RetryActionDiffServer.cc(arms → updateServer(fallback) → no-op on second retry → exception path is caught).HeterogeneousCore/SonicTriton/test/tritonTest_cfg.pywith--retryAction {same,diff}and a verbose confirmation line.TestHeterogeneousCoreSonicTritonRetryActionDiff_Logto assert the selected retry policy.PR validation:
Built and ran unit/integration tests in
CMSSW_15_1_0_pre6area:scram b -j 8TODO/To verify:
scram b runtests TEST=HeterogeneousCore/SonicTriton(passes)Client.Retryis explicitly configured.