-
Notifications
You must be signed in to change notification settings - Fork 298
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
DurableTask.AzureStorage: Alternate fix for the 4 MB max entity size which covers more scenarios. #194
Conversation
// Assume at least 1 KB of data per entity to account for static-length properties | ||
int estimatedByteCount = 1024; | ||
|
||
// Count the bytes for variable-length properties, which are assumed to always be strings |
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 take it this is a safe assumption to make, but I'm curious as to why. Do all property values come in as serialized strings?
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.
Not all, but the ones in the VariableSizeEntityProperties
collection will always be strings. With the exception of Name
(and sometimes Result
), these are all serialized JSON payloads. If we wanted, we could store these as binary data as well, but it's much easier to debug things when they are strings.
EntityProperty property; | ||
if (entity.Properties.TryGetValue(propertyName, out property) && !string.IsNullOrEmpty(property.StringValue)) | ||
{ | ||
estimatedByteCount += Encoding.Unicode.GetByteCount(property.StringValue); |
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.
Curious; why UTF-16? Is this the format Table Storage uses?
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.
Yep. This basically means 2 bytes for every character in a string.
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.
Looks good; couple of general questions about what's going on.
* DurableTask.AzureStorage API to enumerate instances (#187) * DurableTask.AzureStorage ETW trace improvements (#192) * Adding 4MB limit check for Azure Storage Table batch insert (#191) * DurableTask.AzureStorage: Alternate fix for the 4 MB max entity size which covers more scenarios. (#194) * Updated Newtonsoft.Json to v11.0.2, WindowsAzure.Storage to v8.6.0. (#193) * Fixed issues with the ETW event source and added test reliability improvements.
This is a revised version of a previous pull request: #191. After merging, I did some testing and found a few issues. This new version addresses those issues.
FYI @tohling, @gled4er
This addresses Azure/azure-functions-durable-extension#339.