Handle partial write update from GCS #781
Conversation
Connector supports resuming upload only from MAX_BYTES_PER_MESSAGE boundary. If GCS reports a committed offset which is not a multiple of this number, then we cannot resume.
|
/gcbrun |
Codecov Report
@@ Coverage Diff @@
## master #781 +/- ##
=============================================
- Coverage 81.23% 62.89% -18.34%
+ Complexity 1978 1446 -532
=============================================
Files 133 133
Lines 8818 8822 +4
Branches 1025 1026 +1
=============================================
- Hits 7163 5549 -1614
- Misses 1240 2735 +1495
- Partials 415 538 +123
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -220,6 +220,13 @@ private WriteObjectResponse doResumableUpload() throws IOException { | |||
| // Only request committed size for the first insert request. | |||
| if (writeOffset > 0) { | |||
| writeOffset = getCommittedWriteSizeWithRetries(uploadId); | |||
| if (writeOffset % MAX_BYTES_PER_MESSAGE != 0) { | |||
There was a problem hiding this comment.
Please add tests to cover the code path.
Could we get a confirmation from GCS team on the behaviour as well ?
There was a problem hiding this comment.
Also, consider this scenario
- Last chunk of data was writted and committed successfully, but the ack response errored (may be network or client side error)
This is a valid scenario where the commitOffset isn't a multiple of chunk size.
There was a problem hiding this comment.
@veblush In one run of the benchmark we noticed this behavior. Towards the end of the file upload we received a transient error. On trying to resume, we fetched the committedWriteOffset in GCS. It returned a number which was not a multiple of 2MB(MAX_BYTES_PER_MESSAGE). Since we store uncommitted buffers in 2MB, we were not able to resume from that offset.
Can you confirm this?
There was a problem hiding this comment.
Also, consider this scenario
In this case the complete file is written to the GCS. So when we retry,
- We get the file length as the committed Offset.
- We don't have a chunk beginning at that in our buffer, so
- We end up trying to read from the pipe, which returns EOF (0).
- This results in uploading
finalizedchunk to GCS with size as 0.
Connector supports resuming upload only from MAX_BYTES_PER_MESSAGE
boundary. If GCS reports a committed offset which is not a multiple
of this number, then we cannot resume.