Skip to content
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

feat: Add max size check to StreamWriterV2 #873

Merged
merged 6 commits into from Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -22,6 +22,7 @@
import com.google.common.base.Preconditions;
import com.google.common.util.concurrent.Uninterruptibles;
import io.grpc.Status;
import io.grpc.Status.Code;
import io.grpc.StatusRuntimeException;
import java.util.Deque;
import java.util.LinkedList;
Expand All @@ -39,8 +40,6 @@
*
* <p>TODO: Attach schema.
*
* <p>TODO: Add max size check.
*
* <p>TODO: Add inflight control.
*
* <p>TODO: Attach traceId.
Expand Down Expand Up @@ -94,6 +93,11 @@ public class StreamWriterV2 implements AutoCloseable {
*/
private Thread appendThread;

/** The maximum size of one request. Defined by the API. */
public static long getApiMaxRequestBytes() {
return 8L * 1000L * 1000L; // 8 megabytes (https://en.wikipedia.org/wiki/Megabyte)
}

private StreamWriterV2(Builder builder) {
this.lock = new ReentrantLock();
this.hasMessageInWaitingQueue = lock.newCondition();
Expand Down Expand Up @@ -154,6 +158,17 @@ public void run() {
*/
public ApiFuture<AppendRowsResponse> append(AppendRowsRequest message) {
AppendRequestAndResponse requestWrapper = new AppendRequestAndResponse(message);
if (requestWrapper.messageSize > getApiMaxRequestBytes()) {
requestWrapper.appendResult.setException(
new StatusRuntimeException(
Status.fromCode(Code.INVALID_ARGUMENT)
.withDescription(
"MessageSize is too large. Max allow: "
+ getApiMaxRequestBytes()
+ " Actual: "
+ requestWrapper.messageSize)));
return requestWrapper.appendResult;
}
this.lock.lock();
try {
if (userClosed) {
Expand Down Expand Up @@ -355,10 +370,12 @@ public StreamWriterV2 build() {
private static final class AppendRequestAndResponse {
final SettableApiFuture<AppendRowsResponse> appendResult;
final AppendRowsRequest message;
final long messageSize;

AppendRequestAndResponse(AppendRowsRequest message) {
this.appendResult = SettableApiFuture.create();
this.message = message;
this.messageSize = message.getProtoRows().getSerializedSize();
}
}
}
Expand Up @@ -26,6 +26,7 @@
import com.google.api.gax.rpc.ApiException;
import com.google.api.gax.rpc.StatusCode.Code;
import com.google.cloud.bigquery.storage.test.Test.FooType;
import com.google.common.base.Strings;
import com.google.protobuf.DescriptorProtos;
import com.google.protobuf.Int64Value;
import io.grpc.Status;
Expand Down Expand Up @@ -288,6 +289,20 @@ public void serverCloseWhileRequestsInflight() throws Exception {
}

writer.close();
;
}

@Test
public void testMessageTooLarge() {
StreamWriterV2 writer = getTestStreamWriterV2();

String oversized = Strings.repeat("a", (int) (StreamWriterV2.getApiMaxRequestBytes() + 1));
ApiFuture<AppendRowsResponse> appendFuture1 = sendTestMessage(writer, new String[] {oversized});
assertTrue(appendFuture1.isDone());
StatusRuntimeException actualError =
assertFutureException(StatusRuntimeException.class, appendFuture1);
assertEquals(Status.Code.INVALID_ARGUMENT, actualError.getStatus().getCode());
assertTrue(actualError.getStatus().getDescription().contains("MessageSize is too large"));

writer.close();
}
}