-
Notifications
You must be signed in to change notification settings - Fork 425
OAK-12040 - segment-azure: reduce HTTP requests for writes #2665
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
OAK-12040 - segment-azure: reduce HTTP requests for writes #2665
Conversation
ed52177 to
28e7f17
Compare
- avoid copying byte array
28e7f17 to
a288d5c
Compare
| try { | ||
| blob.upload(BinaryData.fromBytes(Arrays.copyOfRange(data, offset, offset + size)), true); | ||
| blob.setMetadata(AzureBlobMetadata.toSegmentMetadata(indexEntry)); | ||
| BinaryData binaryData = BinaryData.fromStream(new ByteArrayInputStream(data, offset, size), (long) size); |
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.
there's quite a bit of complexity behind this, can you just add a comment, e.g.:
| BinaryData binaryData = BinaryData.fromStream(new ByteArrayInputStream(data, offset, size), (long) size); | |
| // upload the binary and set the metadata in a single call to the blobstore | |
| BinaryData binaryData = BinaryData.fromStream(new ByteArrayInputStream(data, offset, size), (long) size); |
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.
Nice finding. Can you outline the measured improvement from that in the ticket?
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.
can you just add a comment
makes sense, done
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.
Nice finding. Can you outline the measured improvement from that in the ticket?
Will do once I get around to measuring something 🙂 However, reducing from 2 to 1 HTTP call cannot make things worse.
From analysis of a log file, I saw that the upload calls had a median of 10ms (and slightly higher avg), while the metadata calls had a median of 8ms (also slightly higher avg). Based on this, I expect a speedup of writes in the range of 30-40%. How that manifests on the JCR level remains to be seen.
Clearly, write-heavy use-cases should benefit most. E.g. I expect compaction to run faster.
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.
@joerghoh I added some performance numbers in the ticket. For a write-heavy use-case I measured an improvement of ~45%.
- add explanatory comment
|



No description provided.