K8SPXC-1542: Fix Azure binlog upload for large files #1848
Conversation
dcaputo-harmoni
requested review from
hors,
egegunes,
inelpandzic and
pooknull
as code owners
|
@dcaputo-harmoni tests are failing because GKE 1.27 is removed. I'll ping you to ask rebase after #1849 merged. |
|
@egegunes No problem, just let me know when |
|
@nmarukovich please create a jira ticket and let's take over this PR. |
|
I added in some proposed changes to make both concurrency and block size configurable for Azure storage endpoints, let me know if this is along the lines of what you were thinking. I'm running into issues now where larger files require even larger block size / concurrency settings so I think this will be useful to be configurable via yaml. |
egegunes
changed the title
commit: 09b4902 |
|
Hi @hors just want to check in on this one, let me know if I can help get this resolved - it's an important feature for anyone running in an Azure production environment.... |
| @@ -227,7 +231,14 @@ func (a *Azure) GetObject(ctx context.Context, name string) (io.ReadCloser, erro | |||
|
|
|||
| func (a *Azure) PutObject(ctx context.Context, name string, data io.Reader, _ int64) error { | |||
| objPath := path.Join(a.prefix, name) | |||
| _, err := a.client.UploadStream(ctx, a.container, objPath, data, nil) | |||
| uploadOptions := azblob.UploadStreamOptions{} | |||
| if a.blockSize > 0 { | |||
There was a problem hiding this comment.
Hello @dcaputo-harmoni
The validations on these fields are not needed. Checking the func (bb *Client) UploadStream(ctx context.Context, body io.Reader, o *UploadStreamOptions) (UploadStreamResponse, error) function, you will notice that at some point the default values both for block size and concurrency are enforced if the assigned values are not the expected.
func (u *UploadStreamOptions) setDefaults() {
if u.Concurrency == 0 {
u.Concurrency = 1
}
if u.BlockSize < _1MiB {
u.BlockSize = _1MiB
}
}
Check this block here: https://github.com/Azure/azure-sdk-for-go/blob/main/sdk/storage/azblob/blockblob/models.go#L337-L345
So, we can safely remove these validations since even if we needed them, it would make more sense to verify them during the NewAzure construction.
So we can do this:
azblob.UploadStreamOptions{
BlockSize: a.blockSize,
Concurrency: a.concurrency,
}
Fixes #1847