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 timeout to inflight queue waiting #1957

Merged
merged 73 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
5a63d95
feat: Split writer into connection worker and wrapper, this is a
GaoleMeng Sep 9, 2022
5a13302
feat: add connection worker pool skeleton, used for multiplexing client
GaoleMeng Sep 13, 2022
0297204
Merge branch 'main' into main
GaoleMeng Sep 14, 2022
8a81ad3
feat: add Load api for connection worker for multiplexing client
GaoleMeng Sep 14, 2022
68fd040
Merge remote-tracking branch 'upstream/main'
GaoleMeng Sep 14, 2022
3106dae
Merge remote-tracking branch 'upstream/main'
GaoleMeng Sep 15, 2022
5bf04e5
Merge branch 'googleapis:main' into main
GaoleMeng Sep 15, 2022
2fc7551
Merge branch 'main' of https://github.com/GaoleMeng/java-bigquerystorage
GaoleMeng Sep 15, 2022
7a6d919
feat: add multiplexing support to connection worker. We will treat every
GaoleMeng Sep 15, 2022
3ba7659
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 16, 2022
f379a78
Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 16, 2022
9307776
Merge branch 'main' of https://github.com/GaoleMeng/java-bigquerystorage
GaoleMeng Sep 16, 2022
de73013
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 16, 2022
19005a1
feat: port the multiplexing client core algorithm and basic tests
GaoleMeng Sep 19, 2022
c5d14ba
Merge branch 'googleapis:main' into main
GaoleMeng Sep 19, 2022
644360a
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 20, 2022
3099d82
Merge branch 'googleapis:main' into main
GaoleMeng Sep 20, 2022
e707dd6
Merge branch 'googleapis:main' into main
GaoleMeng Sep 20, 2022
9e7a8fa
Merge branch 'main' of https://github.com/GaoleMeng/java-bigquerystorage
GaoleMeng Sep 20, 2022
31f1755
Merge branch 'googleapis:main' into main
GaoleMeng Sep 20, 2022
44c36fc
feat: wire multiplexing connection pool to stream writer
GaoleMeng Sep 20, 2022
87a4036
feat: some fixes for multiplexing client
GaoleMeng Sep 23, 2022
c92ea1b
Merge remote-tracking branch 'upstream/main'
GaoleMeng Sep 23, 2022
019520c
Merge branch 'googleapis:main' into main
GaoleMeng Sep 26, 2022
47893df
feat: fix some todos, and reject the mixed behavior of passed in clie…
GaoleMeng Sep 27, 2022
8bd4e6a
Merge remote-tracking branch 'upstream/main'
GaoleMeng Sep 27, 2022
83409b0
Merge remote-tracking branch 'upstream/main'
GaoleMeng Sep 27, 2022
f7dd72d
Merge branch 'googleapis:main' into main
GaoleMeng Sep 27, 2022
a48399f
Merge branch 'googleapis:main' into main
GaoleMeng Sep 29, 2022
6789bc9
feat: fix the bug that we may peek into the write_stream field but it's
GaoleMeng Sep 29, 2022
46b4e6c
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 29, 2022
dfd4dd9
Merge branch 'googleapis:main' into main
GaoleMeng Sep 29, 2022
d68ae70
feat: fix the bug that we may peek into the write_stream field but it's
GaoleMeng Sep 29, 2022
2983fe9
Merge branch 'main' of https://github.com/GaoleMeng/java-bigquerystorage
GaoleMeng Sep 29, 2022
d406256
Merge branch 'googleapis:main' into main
GaoleMeng Oct 13, 2022
22e9e07
feat: add getInflightWaitSeconds implementation
GaoleMeng Oct 13, 2022
fdb4e1c
Merge branch 'googleapis:main' into main
GaoleMeng Oct 21, 2022
0469474
Merge branch 'googleapis:main' into main
GaoleMeng Nov 2, 2022
d1b7740
feat: Add schema comparision in connection loop to ensure schema upda…
GaoleMeng Nov 3, 2022
e4cd529
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Nov 4, 2022
74ff1c4
Merge branch 'googleapis:main' into main
GaoleMeng Nov 4, 2022
762f49e
feat: add schema update support to multiplexing
GaoleMeng Nov 5, 2022
de456c2
Merge branch 'googleapis:main' into main
GaoleMeng Nov 11, 2022
c2f6edc
Merge branch 'googleapis:main' into main
GaoleMeng Nov 15, 2022
2487227
fix: fix windows build bug: windows Instant resolution is different with
GaoleMeng Nov 15, 2022
084d6d1
fix: fix another failing tests for windows build
GaoleMeng Nov 16, 2022
89c9701
Merge branch 'main' of https://github.com/GaoleMeng/java-bigquerystorage
GaoleMeng Nov 16, 2022
8441518
fix: fix another test failure for Windows build
GaoleMeng Nov 16, 2022
d249add
Merge branch 'googleapis:main' into main
GaoleMeng Nov 30, 2022
83aa7ff
feat: Change new thread for each retry to be a thread pool to avoid
GaoleMeng Nov 30, 2022
92a9c36
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Nov 30, 2022
a713a52
Merge branch 'googleapis:main' into main
GaoleMeng Nov 30, 2022
a042d5c
fix: add back the background executor provider that's accidentally
GaoleMeng Nov 30, 2022
53f4ec8
feat: throw error when use connection pool for explicit stream
GaoleMeng Dec 2, 2022
c494d8b
Merge branch 'googleapis:main' into main
GaoleMeng Dec 20, 2022
14b0c12
fix: Add precision truncation to the passed in value from JSON float and
GaoleMeng Jan 17, 2023
0da0e4b
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jan 17, 2023
33d23ac
Merge branch 'googleapis:main' into main
GaoleMeng Jan 17, 2023
d2ee46e
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jan 17, 2023
be6646e
modify the bom version
GaoleMeng Jan 17, 2023
62d8c41
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jan 17, 2023
adf5f3f
fix deadlockissue in ConnectionWorkerPool
GaoleMeng Jan 18, 2023
c1970ff
Merge branch 'googleapis:main' into main
GaoleMeng Jan 18, 2023
3488df8
fix: fix deadlock issue during close + append for multiplexing
GaoleMeng Jan 19, 2023
6a512e8
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jan 20, 2023
05edc2f
Merge branch 'googleapis:main' into main
GaoleMeng Jan 20, 2023
7d3da74
Merge branch 'googleapis:main' into main
GaoleMeng Jan 20, 2023
ecf6807
Merge branch 'googleapis:main' into main
GaoleMeng Jan 20, 2023
057dab9
Merge branch 'googleapis:main' into main
GaoleMeng Jan 23, 2023
5db46a2
fix: fix one potential root cause of deadlock issue for non-multiplexing
GaoleMeng Jan 23, 2023
32e9d33
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jan 25, 2023
183941a
Merge branch 'googleapis:main' into main
GaoleMeng Jan 25, 2023
f93f89e
Add timeout to inflight queue waiting, and also add some extra log
GaoleMeng Jan 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@
public class ConnectionWorker implements AutoCloseable {
private static final Logger log = Logger.getLogger(StreamWriter.class.getName());

// Maximum wait time on inflight quota before error out.
private static long INFLIGHT_QUOTA_MAX_WAIT_TIME_MILLI = 300000;

private Lock lock;
private Condition hasMessageInWaitingQueue;
private Condition inflightReduced;
Expand Down Expand Up @@ -322,7 +325,14 @@ private ApiFuture<AppendRowsResponse> appendInternal(AppendRowsRequest message)
this.inflightBytes += requestWrapper.messageSize;
waitingRequestQueue.addLast(requestWrapper);
hasMessageInWaitingQueue.signal();
maybeWaitForInflightQuota();
try {
maybeWaitForInflightQuota();
} catch (StatusRuntimeException ex) {
--this.inflightRequests;
waitingRequestQueue.pollLast();
this.inflightBytes -= requestWrapper.messageSize;
throw ex;
}
return requestWrapper.appendResult;
} finally {
this.lock.unlock();
Expand All @@ -347,6 +357,15 @@ private void maybeWaitForInflightQuota() {
.withCause(e)
.withDescription("Interrupted while waiting for quota."));
}
long current_wait_time = System.currentTimeMillis() - start_time;
if (current_wait_time > INFLIGHT_QUOTA_MAX_WAIT_TIME_MILLI) {
throw new StatusRuntimeException(
Status.fromCode(Code.CANCELLED)
.withDescription(
String.format(
"Interrupted while waiting for quota due to long waiting time %sms",
current_wait_time)));
}
}
inflightWaitSec.set((System.currentTimeMillis() - start_time) / 1000);
}
Expand All @@ -373,7 +392,6 @@ public void close() {
log.fine("Waiting for append thread to finish. Stream: " + streamName);
try {
appendThread.join();
log.info("User close complete. Stream: " + streamName);
} catch (InterruptedException e) {
// Unexpected. Just swallow the exception with logging.
log.warning(
Expand All @@ -387,6 +405,7 @@ public void close() {
}

try {
log.fine("Begin shutting down user callback thread pool for stream " + streamName);
threadPool.shutdown();
threadPool.awaitTermination(3, TimeUnit.MINUTES);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log when we actually timed out here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

} catch (InterruptedException e) {
Expand All @@ -396,7 +415,10 @@ public void close() {
+ streamName
+ " is interrupted with exception: "
+ e.toString());
throw new IllegalStateException(
"Thread pool shutdown is interrupted for stream " + streamName);
}
log.info("User close finishes for stream " + streamName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the line on 395?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

/*
Expand Down Expand Up @@ -858,6 +880,11 @@ public static void setOverwhelmedCountsThreshold(double newThreshold) {
}
}

@VisibleForTesting
static void setMaxInflightQueueWaitTime(long waitTime) {
INFLIGHT_QUOTA_MAX_WAIT_TIME_MILLI = waitTime;
}

@AutoValue
abstract static class TableSchemaAndTimestamp {
// Shows the timestamp updated schema is reported from response
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
package com.google.cloud.bigquery.storage.v1;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import com.google.api.core.ApiFuture;
import com.google.api.gax.batching.FlowController;
Expand All @@ -28,7 +31,9 @@
import com.google.cloud.bigquery.storage.v1.ConnectionWorker.Load;
import com.google.protobuf.DescriptorProtos;
import com.google.protobuf.Int64Value;
import io.grpc.StatusRuntimeException;
import java.io.IOException;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand All @@ -52,6 +57,7 @@ public class ConnectionWorkerTest {
@Before
public void setUp() throws Exception {
testBigQueryWrite = new FakeBigQueryWrite();
ConnectionWorker.setMaxInflightQueueWaitTime(300000);
serviceHelper =
new MockServiceHelper(
UUID.randomUUID().toString(), Arrays.<MockGrpcService>asList(testBigQueryWrite));
Expand Down Expand Up @@ -281,6 +287,63 @@ public void testAppendInSameStream_switchSchema() throws Exception {
}
}

@Test
public void testAppendButInflightQueueFull() throws Exception {
ConnectionWorker connectionWorker =
new ConnectionWorker(
TEST_STREAM_1,
createProtoSchema("foo"),
6,
100000,
Duration.ofSeconds(100),
FlowController.LimitExceededBehavior.Block,
TEST_TRACE_ID,
client.getSettings());
testBigQueryWrite.setResponseSleep(org.threeten.bp.Duration.ofSeconds(1));
ConnectionWorker.setMaxInflightQueueWaitTime(500);
ProtoSchema schema1 = createProtoSchema("foo");

long appendCount = 6;
for (int i = 0; i < appendCount; i++) {
testBigQueryWrite.addResponse(createAppendResponse(i));
}

// In total insert 6 requests, since the max queue size is 5 we will stuck at the 6th request.
List<ApiFuture<AppendRowsResponse>> futures = new ArrayList<>();
for (int i = 0; i < appendCount; i++) {
long startTime = System.currentTimeMillis();
// At the last request we wait more than 500 millisecond for inflight quota.
if (i == 5) {
assertThrows(
StatusRuntimeException.class,
() -> {
sendTestMessage(
connectionWorker,
TEST_STREAM_1,
schema1,
createFooProtoRows(new String[] {String.valueOf(5)}),
5);
});
long timeDiff = System.currentTimeMillis() - startTime;
assertEquals(connectionWorker.getLoad().inFlightRequestsCount(), 5);
assertTrue(timeDiff > 500);
} else {
futures.add(
sendTestMessage(
connectionWorker,
TEST_STREAM_1,
schema1,
createFooProtoRows(new String[] {String.valueOf(i)}),
i));
assertEquals(connectionWorker.getLoad().inFlightRequestsCount(), i + 1);
}
}

for (int i = 0; i < appendCount - 1; i++) {
assertEquals(i, futures.get(i).get().getAppendResult().getOffset().getValue());
}
}

private AppendRowsResponse createAppendResponse(long offset) {
return AppendRowsResponse.newBuilder()
.setAppendResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public StreamWriterTest() throws DescriptorValidationException {}
@Before
public void setUp() throws Exception {
testBigQueryWrite = new FakeBigQueryWrite();
ConnectionWorker.setMaxInflightQueueWaitTime(300000);
serviceHelper =
new MockServiceHelper(
UUID.randomUUID().toString(), Arrays.<MockGrpcService>asList(testBigQueryWrite));
Expand Down