Retry for ingestion Resources #356
Conversation
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 674b6a9. ♻️ This comment has been updated with latest results. |
debug prints
…to-spark into feature/retryTempStorage
| @@ -75,17 +75,20 @@ class ContainerProvider(val client: ExtendedKustoClient, val clusterAlias: Strin | |||
| storageUris(roundRobinIdx) | |||
| } | |||
| } else { | |||
| Try(client.ingestClient.getResourceManager.getShuffledContainers) match { | |||
There was a problem hiding this comment.
Lets not throw RuntimeException create a new exception or use something that extends Exception as this is a recoverable exception- Also to me this looks like a bug in our Java SDK - because this means we returned here without error and got empty resources
| @@ -0,0 +1,36 @@ | |||
| name: Build Java | |||
There was a problem hiding this comment.
how is this related to the PR?
There was a problem hiding this comment.
with these changes we are moving away from AzCi to github actions
There was a problem hiding this comment.
just next time - separate PRs
|
|
||
| val kustoOperationResult = new KustoOperationResult(readTestSource("storage-result-empty.json"), "v1") | ||
| val mockDmClient = mock[Client] | ||
| // (mockDmClient.execute(_: String, _: String, _: ClientRequestProperties)).expects(*, *, *). |
| val retryException: Predicate[Throwable] = (e: Throwable) => | ||
| (e.isInstanceOf[IngestionServiceException] && !e.asInstanceOf[KustoDataExceptionBase].isPermanent) || | ||
| (e.isInstanceOf[DataServiceException] && ExceptionUtils.getRootCause(e).isInstanceOf[HttpHostConnectException]) | ||
| private val retryConfig = buildRetryConfig((e: Throwable) => |
There was a problem hiding this comment.
| private val retryConfig = buildRetryConfig((e: Throwable) => | |
| private val retryConfigExportContainers = buildRetryConfig((e: Throwable) => |
| private val retryConfig = buildRetryConfig((e: Throwable) => | ||
| (e.isInstanceOf[IngestionServiceException] && !e.asInstanceOf[KustoDataExceptionBase].isPermanent) || | ||
| (e.isInstanceOf[DataServiceException] && ExceptionUtils.getRootCause(e).isInstanceOf[HttpHostConnectException])) | ||
| private val retryConfigIngestionRefresh = buildRetryConfig(e => e.isInstanceOf[RuntimeException]) |
There was a problem hiding this comment.
After we change it to a different type of exception change it here as well and add here all the SDK exceptions as well
There was a problem hiding this comment.
Still here RuntimeException - need to be only the IngestionServiceException, IngestionClientException
There was a problem hiding this comment.
Looks like outdated code.
see here
Added new exception
| @@ -27,7 +28,7 @@ class ContainerProviderTest extends AnyFlatSpec with Matchers with MockFactory { | |||
|
|
|||
| maybeExceptionThrown match { | |||
| case Some(exception) => Mockito.when(mockIngestionResourceManager.getShuffledContainers) | |||
| .thenThrow(exception).thenThrow(exception). | |||
| .thenThrow(exception, exception, exception, exception, exception, exception, exception, exception). //throws exception 8 times due to retry | |||
There was a problem hiding this comment.
Gave good amount of time to simplify this but no luck 😐
| private val retryConfig = buildRetryConfig((e: Throwable) => | ||
| (e.isInstanceOf[IngestionServiceException] && !e.asInstanceOf[KustoDataExceptionBase].isPermanent) || | ||
| (e.isInstanceOf[DataServiceException] && ExceptionUtils.getRootCause(e).isInstanceOf[HttpHostConnectException])) | ||
| private val retryConfigIngestionRefresh = buildRetryConfig(e => e.isInstanceOf[RuntimeException]) |
There was a problem hiding this comment.
Still here RuntimeException - need to be only the IngestionServiceException, IngestionClientException
* Retry for ingestion Resources (#356) * Remove irrelvant tests * Fix ignored tests * Add build file * Added Retry for refresh ingestion * Code changes for TCs * ghCI change --------- Co-authored-by: Ramachandran A G <[email protected]> * code change for 2.4 * mockito syntax change for scala 2.11 --------- Co-authored-by: Ramachandran A G <[email protected]>
Pull Request Description
Added retry mechanism for the cases when no storage is returned.
errortrace:
Future Release Comment
[Add description of your change, to include in the next release]
[Delete any or all irrelevant sections, e.g. if your change does not warrant a release comment at all]
Breaking Changes:
Features:
Fixes: