Skip to content

SAMZA-2699: AzureBlob SystemProducer: remove retries when uploading block as azure sdk does internal retries#1542

Merged
mynameborat merged 1 commit intoapache:masterfrom
lakshmi-manasa-g:azure-system-producer-remove-retry
Oct 14, 2021
Merged

SAMZA-2699: AzureBlob SystemProducer: remove retries when uploading block as azure sdk does internal retries#1542
mynameborat merged 1 commit intoapache:masterfrom
lakshmi-manasa-g:azure-system-producer-remove-retry

Conversation

@lakshmi-manasa-g
Copy link
Contributor

Symptom: interrupted thread stays stuck for a long time when interrupted exception is wrapped in another exception.

Cause: retry several times for all exceptions during block upload. though InterruptedException was immediately bubbled up in #1511, when wrapped it is not bubbled.

Changes: remove retries at samza level since azure sdk does the retries.
Samza uses BlobServiceClientBuilder which uses RequestRetryOptions as the default retry policy and the RequestRetryOptions sets max retries to 4 with exponential backoff.

Tests: removing retry.
API changes: None
Usage/upgrade instructions: N/A
Backwards compatible: yes

ByteBuffer outputStream = ByteBuffer.wrap(compressedLocalByte, 0, blockSize);
metrics.updateCompressByteMetrics(blockSize);
LOG.info("{} Upload block start for blob: {} for block size:{}.", blobAsyncClient.getBlobUrl().toString(), blockId, blockSize);
metrics.updateAzureUploadMetrics();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should metric be updated after upload is complete, i.e. stageBlock?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing so now will actually make this change backwards incompatible.
the errors for upload are tracked separately here

Copy link
Contributor

@rmatharu-zz rmatharu-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metric update

Copy link
Contributor

@ZitingShen ZitingShen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mynameborat mynameborat merged commit 45c0aa9 into apache:master Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants