-
Notifications
You must be signed in to change notification settings - Fork 26
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
Support automatic resizing for variable-size page blobs #329
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,8 +62,6 @@ struct RemoveRequestInfo | |
const uint MAX_UPLOAD_SIZE = 1024 * 1024; | ||
const uint MAX_DOWNLOAD_SIZE = 1024 * 1024; | ||
|
||
const long MAX_PAGEBLOB_SIZE = 512L * 1024 * 1024 * 1024; // set this at 512 GB for now TODO consider implications | ||
|
||
/// <summary> | ||
/// Constructs a new AzureStorageDevice instance, backed by Azure Page Blobs | ||
/// </summary> | ||
|
@@ -424,8 +422,8 @@ public override void WriteAsync(IntPtr sourceAddress, int segmentId, ulong desti | |
{ | ||
var pageBlob = this.pageBlobDirectory.GetPageBlobClient(this.GetSegmentBlobName(segmentId)); | ||
|
||
// If segment size is -1 we use a default | ||
var size = this.segmentSize == -1 ? AzureStorageDevice.MAX_PAGEBLOB_SIZE : this.segmentSize; | ||
// If segment size is -1 we use the default starting size for auto-expanding page blobs. | ||
var size = this.segmentSize == -1 ? this.BlobManager.StartingPageBlobSize : this.segmentSize; | ||
|
||
// If no blob exists for the segment, we must first create the segment asynchronouly. (Create call takes ~70 ms by measurement) | ||
// After creation is done, we can call write. | ||
|
@@ -469,15 +467,55 @@ await this.BlobManager.PerformWithRetriesAsync( | |
{ | ||
var client = numAttempts > 2 ? blobEntry.PageBlob.Default : blobEntry.PageBlob.Aggressive; | ||
|
||
var response = await client.UploadPagesAsync( | ||
content: stream, | ||
offset: destinationAddress + offset, | ||
transactionalContentHash: null, | ||
conditions: this.underLease ? new PageBlobRequestConditions() { IfMatch = blobEntry.ETag } : null, | ||
progressHandler: null, | ||
cancellationToken: this.PartitionErrorHandler.Token).ConfigureAwait(false); | ||
try | ||
{ | ||
var response = await client.UploadPagesAsync( | ||
content: stream, | ||
offset: destinationAddress + offset, | ||
transactionalContentHash: null, | ||
conditions: this.underLease ? new PageBlobRequestConditions() { IfMatch = blobEntry.ETag } : null, | ||
progressHandler: null, | ||
cancellationToken: this.PartitionErrorHandler.Token).ConfigureAwait(false); | ||
|
||
blobEntry.ETag = response.Value.ETag; | ||
} | ||
catch (Azure.RequestFailedException e) when (e.ErrorCode == "InvalidPageRange") | ||
{ | ||
// this kind of error can indicate that the page blob is too small. | ||
// from the perspective of FASTER, this storage device is infinite, so it may write past the end of the blob. | ||
// To deal with this situation, we dynamically enlarge this device as needed. | ||
|
||
// first, compute desired size to request | ||
long currentSize = (await client.GetPropertiesAsync().ConfigureAwait(false)).Value.ContentLength; | ||
long sizeToRequest = currentSize; | ||
long sizeToAccommodate = destinationAddress + offset + length + 1; | ||
while (sizeToAccommodate > sizeToRequest) | ||
{ | ||
sizeToRequest <<= 1; | ||
} | ||
|
||
blobEntry.ETag = response.Value.ETag; | ||
if (sizeToRequest <= currentSize) | ||
{ | ||
throw e; // blob is already big enough, so this exception was thrown for some other reason | ||
} | ||
else | ||
{ | ||
if (sizeToRequest > this.BlobManager.MaxPageBlobSize) | ||
{ | ||
throw new InvalidOperationException($"cannot expand page blob {blobEntry.PageBlob.Default.Name} beyond maximum size {this.BlobManager.MaxPageBlobSize}"); | ||
} | ||
|
||
// enlarge the blob to accommodate the size | ||
await client.ResizeAsync( | ||
sizeToRequest, | ||
conditions: this.underLease ? new PageBlobRequestConditions() { IfMatch = blobEntry.ETag } : null, | ||
cancellationToken: this.PartitionErrorHandler.Token).ConfigureAwait(false); | ||
|
||
// force retry | ||
// this also generates a warning in the traces, containing the information about what happened | ||
throw new BlobUtils.ForceRetryException($"page blob was enlarged from {currentSize} to {sizeToRequest}", e); | ||
} | ||
} | ||
} | ||
|
||
return (long)length; | ||
|
@@ -500,11 +538,17 @@ async Task ReadFromBlobAsync(UnmanagedMemoryStream stream, BlobUtilsV12.PageBlob | |
{ | ||
using (stream) | ||
{ | ||
// we use this to prevent reading past the end of the page blob | ||
// but for performance reasons (Azure storage access required to determine current size of page blob) | ||
// we set it lazily, i.e. only after a request failed | ||
long? readCap = null; | ||
|
||
long offset = 0; | ||
while (readLength > 0) | ||
{ | ||
var length = Math.Min(readLength, MAX_DOWNLOAD_SIZE); | ||
|
||
// determine how much we are going to try to read in this portion | ||
var length = Math.Min(readLength, MAX_DOWNLOAD_SIZE); | ||
|
||
await this.BlobManager.PerformWithRetriesAsync( | ||
BlobManager.AsynchronousStorageReadMaxConcurrency, | ||
true, | ||
|
@@ -516,36 +560,94 @@ await this.BlobManager.PerformWithRetriesAsync( | |
true, | ||
async (numAttempts) => | ||
{ | ||
if (numAttempts > 0) | ||
{ | ||
stream.Seek(offset, SeekOrigin.Begin); // must go back to original position before retrying | ||
} | ||
stream.Seek(offset, SeekOrigin.Begin); | ||
|
||
if (length > 0) | ||
{ | ||
var client = (numAttempts > 1 || length == MAX_DOWNLOAD_SIZE) ? blob.Default : blob.Aggressive; | ||
long requestedLength = length; | ||
|
||
var response = await client.DownloadStreamingAsync( | ||
range: new Azure.HttpRange(sourceAddress + offset, length), | ||
conditions: null, | ||
rangeGetContentHash: false, | ||
cancellationToken: this.PartitionErrorHandler.Token) | ||
.ConfigureAwait(false); | ||
if (readCap.HasValue && sourceAddress + offset + requestedLength > readCap.Value) | ||
{ | ||
requestedLength = readCap.Value - (sourceAddress + offset); | ||
|
||
using (var streamingResult = response.Value) | ||
if (requestedLength <= 0) | ||
{ | ||
await streamingResult.Content.CopyToAsync(stream).ConfigureAwait(false); | ||
requestedLength = 0; | ||
} | ||
} | ||
|
||
if (stream.Position != offset + length) | ||
if (requestedLength > 0) | ||
{ | ||
throw new InvalidDataException($"wrong amount of data received from page blob, expected={length}, actual={stream.Position}"); | ||
var client = (numAttempts > 1 || requestedLength == MAX_DOWNLOAD_SIZE) ? blob.Default : blob.Aggressive; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you comment on what this controls? What is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. they control the length of the timeout (with newer storage SDK the timeout can no longer be passed as a parameter to the operation, since it is a constructor parameter for the client, so we need multiple clients to control the timeout length). BTW this was not modified in this PR, the diff just does not show this well. |
||
|
||
try | ||
{ | ||
var response = await client.DownloadStreamingAsync( | ||
range: new Azure.HttpRange(sourceAddress + offset, requestedLength), | ||
conditions: null, | ||
rangeGetContentHash: false, | ||
cancellationToken: this.PartitionErrorHandler.Token) | ||
.ConfigureAwait(false); | ||
|
||
using (var streamingResult = response.Value) | ||
{ | ||
await streamingResult.Content.CopyToAsync(stream).ConfigureAwait(false); | ||
} | ||
|
||
// We have observed that we may get 206 (Partial Response) codes where the actual length is less than the requested length | ||
// The Azure storage client SDK handles the http codes transparently, but we may still observe that fewer bytes than | ||
// requested were returned by the streamingResult. | ||
long actualLength = (stream.Position - offset); | ||
|
||
if (actualLength < requestedLength) | ||
{ | ||
this.BlobManager.StorageTracer?.FasterStorageProgress($"PageBlob.DownloadStreamingAsync id={id} returned partial response range={response.Value.Details.ContentRange} requestedLength={requestedLength} actualLength={actualLength}"); | ||
|
||
if (actualLength == 0) | ||
{ | ||
throw new InvalidDataException($"PageBlob.DownloadStreamingAsync returned empty response, range={response.Value.Details.ContentRange} requestedLength={requestedLength} "); | ||
} | ||
else if (actualLength % 512 != 0) | ||
{ | ||
throw new InvalidDataException($"PageBlob.DownloadStreamingAsync returned unaligned response, range={response.Value.Details.ContentRange} requestedLength={requestedLength} actualLength=${actualLength}"); | ||
} | ||
else | ||
{ | ||
length = (uint)actualLength; // adjust length to actual length read so the next read will start where this read ended | ||
} | ||
} | ||
else if (actualLength > requestedLength) | ||
{ | ||
throw new InvalidDataException($"PageBlob.DownloadStreamingAsync returned too much data, range={response.Value.Details.ContentRange} requestedLength={requestedLength} actualLength=${actualLength}"); | ||
} | ||
} | ||
catch (Azure.RequestFailedException e) when (e.ErrorCode == "InvalidRange") | ||
{ | ||
// from the perspective of FASTER, this storage device is infinite, so it may read past the end of the blob. | ||
// But even though it requests more data than what it wrote, it will only actually use what it wrote before. | ||
// so we can deal with this situation by just copying fewer bytes from the blob into the buffer. | ||
|
||
// first, determine current page blob size. | ||
var properties = await client.GetPropertiesAsync().ConfigureAwait(false); | ||
readCap = properties.Value.ContentLength; | ||
|
||
if (sourceAddress + offset + requestedLength <= readCap.Value) | ||
{ | ||
// page blob is big enough, so this exception was thrown for some other reason | ||
throw e; | ||
} | ||
else | ||
{ | ||
// page blob was indeed too small; now that we have set a read cap, force a retry | ||
// so we can read an adjusted portion | ||
throw new BlobUtils.ForceRetryException($"reads now capped at {readCap}", e); | ||
} | ||
} | ||
} | ||
|
||
return length; | ||
}); | ||
|
||
// adjust how much we have to read, and where to read from, in the next iteration | ||
// based on how much was actually read in this iteration. | ||
readLength -= length; | ||
offset += length; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't immediately understand why enlarging the blobs can suddenly cause reads to go past the end of the page blob. Can you please call out the connection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because these blobs don't actually have a fixed size from the perspective of FASTER, the FASTER reads may easily read past the end of the blob. This is not new - we just never saw it because 512GB was always way larger than needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think it would be good to write that in the inlined comment being referenced here. This also answers my question here: #329 (comment)