Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolving 500 server timeouts on append blob appends #2430

Merged
merged 22 commits into from Nov 29, 2023

Conversation

gapra-msft
Copy link
Member

@gapra-msft gapra-msft commented Oct 26, 2023

Tested GCP manually since pipeline GCP creds are not working

@gapra-msft gapra-msft changed the title Gapra/resolve500 timeout Resolving 500 server timeouts on apppend blob appends Oct 26, 2023
@gapra-msft gapra-msft changed the title Resolving 500 server timeouts on apppend blob appends Resolving 500 server timeouts on append blob appends Oct 27, 2023
@@ -68,14 +68,18 @@ func (u *appendBlobUploader) GenerateUploadFunc(id common.ChunkID, blockIndex in
u.jptm.LogChunkStatus(id, common.EWaitReason.Body())
body := newPacedRequestBody(u.jptm.Context(), reader, u.pacer)
offset := id.OffsetInFile()
_, err := u.destAppendBlobClient.AppendBlock(u.jptm.Context(), body,
var timeoutFromCtx bool
ctx := withTimeoutNotification(u.jptm.Context(), &timeoutFromCtx)
Copy link
Member

Choose a reason for hiding this comment

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

why do we need context with timeout here, in older code its different right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is actually a notifier that a 500 status code was returned during the append block retries so we can check below. This is not adding a request timeout

@nakulkar-msft nakulkar-msft added this to the 10.22 milestone Nov 29, 2023
@gapra-msft gapra-msft merged commit 97a35fd into dev Nov 29, 2023
17 checks passed
@gapra-msft gapra-msft deleted the gapra/resolve500Timeout branch November 29, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants