Skip to content

Commit

Permalink
fix: improve error detection and reporting for BlobWriteChannel retry…
Browse files Browse the repository at this point in the history
… state (#846)

Add new checks to ensure a more informative error than NullPointerException is thrown if the StorageObject or it's size are unable to be resolved on the last chunk.

Fixes #839
  • Loading branch information
BenWhitehead committed Jun 1, 2021
1 parent df98af2 commit d0f2184
Showing 1 changed file with 82 additions and 10 deletions.
Expand Up @@ -26,6 +26,7 @@
import com.google.cloud.WriteChannel;
import com.google.cloud.storage.spi.v1.StorageRpc;
import com.google.common.collect.Maps;
import java.math.BigInteger;
import java.net.URL;
import java.util.Map;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -83,14 +84,52 @@ private StorageObject getRemoteStorageObject() {
.get(getEntity().toPb(), Maps.newEnumMap(StorageRpc.Option.class));
}

private StorageException unrecoverableState(
int chunkOffset, int chunkLength, long localPosition, long remotePosition, boolean last) {
private static StorageException unrecoverableState(
String uploadId,
int chunkOffset,
int chunkLength,
long localPosition,
long remotePosition,
boolean last) {
return unrecoverableState(
uploadId,
chunkOffset,
chunkLength,
localPosition,
remotePosition,
last,
"Unable to recover in upload.\nThis may be a symptom of multiple clients uploading to the same upload session.");
}

private static StorageException errorResolvingMetadataLastChunk(
String uploadId,
int chunkOffset,
int chunkLength,
long localPosition,
long remotePosition,
boolean last) {
return unrecoverableState(
uploadId,
chunkOffset,
chunkLength,
localPosition,
remotePosition,
last,
"Unable to load object metadata to determine if last chunk was successfully written");
}

private static StorageException unrecoverableState(
String uploadId,
int chunkOffset,
int chunkLength,
long localPosition,
long remotePosition,
boolean last,
String message) {
StringBuilder sb = new StringBuilder();
sb.append("Unable to recover in upload.\n");
sb.append(
"This may be a symptom of multiple clients uploading to the same upload session.\n\n");
sb.append(message).append("\n\n");
sb.append("For debugging purposes:\n");
sb.append("uploadId: ").append(getUploadId()).append('\n');
sb.append("uploadId: ").append(uploadId).append('\n');
sb.append("chunkOffset: ").append(chunkOffset).append('\n');
sb.append("chunkLength: ").append(chunkLength).append('\n');
sb.append("localOffset: ").append(localPosition).append('\n');
Expand Down Expand Up @@ -162,7 +201,7 @@ public void run() {
// Get remote offset from API
final long localPosition = getPosition();
// For each request it should be possible to retry from its location in this code
final long remotePosition = isRetrying() ? getRemotePosition() : getPosition();
final long remotePosition = isRetrying() ? getRemotePosition() : localPosition;
final int chunkOffset = (int) (remotePosition - localPosition);
final int chunkLength = length - chunkOffset;
final boolean uploadAlreadyComplete = remotePosition == -1;
Expand All @@ -176,10 +215,38 @@ public void run() {
if (storageObject == null) {
storageObject = getRemoteStorageObject();
}
// the following checks are defined here explicitly to provide a more
// informative if either storageObject is unable to be resolved or it's size is
// unable to be determined. This scenario is a very rare case of failure that
// can arise when packets are lost.
if (storageObject == null) {
throw errorResolvingMetadataLastChunk(
getUploadId(),
chunkOffset,
chunkLength,
localPosition,
remotePosition,
lastChunk);
}
// Verify that with the final chunk we match the blob length
if (storageObject.getSize().longValue() != getPosition() + length) {
BigInteger size = storageObject.getSize();
if (size == null) {
throw errorResolvingMetadataLastChunk(
getUploadId(),
chunkOffset,
chunkLength,
localPosition,
remotePosition,
lastChunk);
}
if (size.longValue() != getPosition() + length) {
throw unrecoverableState(
chunkOffset, chunkLength, localPosition, remotePosition, lastChunk);
getUploadId(),
chunkOffset,
chunkLength,
localPosition,
remotePosition,
lastChunk);
}
retrying = false;
} else if (uploadAlreadyComplete && !lastChunk && !checkingForLastChunk) {
Expand All @@ -201,7 +268,12 @@ public void run() {
} else {
// Case 4 && Case 8 && Case 9
throw unrecoverableState(
chunkOffset, chunkLength, localPosition, remotePosition, lastChunk);
getUploadId(),
chunkOffset,
chunkLength,
localPosition,
remotePosition,
lastChunk);
}
}
}),
Expand Down

0 comments on commit d0f2184

Please sign in to comment.