Skip to content

Commit 668abf7

Browse files
author
Dhriti Chopra
committed
chore(storage): remove the fast open prototype
Fifth step of fast open BiDi reads (go/fast-open-bidi-read, b/552263062). StorageDataClient.readSession(req, ctx, fastOpenConfigs) and the session's fast open read now do what the single-range prototype did, so remove fastOpenReadSession, FastOpenObjectReadSession and the Borrowable lease on ObjectReadSessionStream, which only the prototype used. The prototype's tests move to the real path: they open with readSession and read with readAs, keeping fast open coverage for byte array, ByteString and channel reads.
1 parent 21b2615 commit 668abf7

4 files changed

Lines changed: 14 additions & 120 deletions

File tree

‎java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ObjectReadSessionStream.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import com.google.cloud.storage.ObjectReadSessionState.OpenArguments;
3232
import com.google.cloud.storage.ResponseContentLifecycleHandle.ChildRef;
3333
import com.google.cloud.storage.RetryContext.OnSuccess;
34-
import com.google.cloud.storage.StorageDataClient.Borrowable;
3534
import com.google.common.annotations.VisibleForTesting;
3635
import com.google.common.base.Preconditions;
3736
import com.google.protobuf.ByteString;
@@ -58,7 +57,7 @@
5857
import org.checkerframework.checker.nullness.qual.Nullable;
5958

6059
final class ObjectReadSessionStream
61-
implements ClientStream<BidiReadObjectRequest>, ApiFuture<Void>, IOAutoCloseable, Borrowable {
60+
implements ClientStream<BidiReadObjectRequest>, ApiFuture<Void>, IOAutoCloseable {
6261

6362
private final SettableApiFuture<Void> objectReadSessionResolveFuture;
6463

@@ -206,11 +205,6 @@ boolean isOpen() {
206205
return openLeases.get() > 0;
207206
}
208207

209-
public void borrow() {
210-
checkOpen();
211-
openLeases.incrementAndGet();
212-
}
213-
214208
private void checkOpen() {
215209
Preconditions.checkState(isOpen(), "Stream closed");
216210
}

‎java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageDataClient.java‎

Lines changed: 0 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,11 @@
2323
import com.google.api.core.InternalApi;
2424
import com.google.api.gax.grpc.GrpcCallContext;
2525
import com.google.cloud.storage.GrpcUtils.ZeroCopyBidiStreamingCallable;
26-
import com.google.cloud.storage.ObjectReadSessionState.OpenArguments;
2726
import com.google.cloud.storage.ReadProjectionConfig.ProjectionType;
2827
import com.google.cloud.storage.RetryContext.RetryContextProvider;
2928
import com.google.common.collect.ImmutableList;
30-
import com.google.common.collect.ImmutableMap;
3129
import com.google.storage.v2.BidiReadObjectRequest;
3230
import com.google.storage.v2.BidiReadObjectResponse;
33-
import java.io.IOException;
3431
import java.time.Duration;
3532
import java.util.HashMap;
3633
import java.util.List;
@@ -117,46 +114,6 @@ ApiFuture<ObjectReadSession> readSession(
117114
return objectReadSessionFuture;
118115
}
119116

120-
<Projection> ApiFuture<FastOpenObjectReadSession<Projection>> fastOpenReadSession(
121-
BidiReadObjectRequest openRequest,
122-
GrpcCallContext ctx,
123-
ReadProjectionConfig<Projection> config) {
124-
checkArgument(
125-
openRequest.getReadRangesList().isEmpty(),
126-
"ranges included in the initial request are not supported");
127-
checkArgument(
128-
config.getType() == ProjectionType.STREAM_READ,
129-
"unsupported ReadProjectionConfig: %s",
130-
config.getClass().getName());
131-
ObjectReadSessionState state = new ObjectReadSessionState(ctx, openRequest);
132-
133-
ZeroCopyBidiStreamingCallable<BidiReadObjectRequest, BidiReadObjectResponse> callable =
134-
getCallable();
135-
ObjectReadSessionStream stream =
136-
ObjectReadSessionStream.create(executor, callable, state, retryContextProvider.create());
137-
138-
long readId = state.newReadId();
139-
ObjectReadSessionStreamRead<Projection> read =
140-
config.cast().newRead(readId, retryContextProvider.create());
141-
state.putOutstandingRead(readId, read);
142-
143-
ApiFuture<FastOpenObjectReadSession<Projection>> objectReadSessionFuture =
144-
ApiFutures.transform(
145-
stream,
146-
nowOpen ->
147-
new FastOpenObjectReadSession<>(
148-
new ObjectReadSessionImpl(
149-
executor, callable, stream, state, retryContextProvider, ImmutableMap.of()),
150-
read,
151-
stream),
152-
executor);
153-
OpenArguments openArguments = state.getOpenArguments();
154-
BidiReadObjectRequest req = openArguments.getReq();
155-
stream.send(req);
156-
read.setOnCloseCallback(stream);
157-
return objectReadSessionFuture;
158-
}
159-
160117
@SuppressWarnings("ResultOfMethodCallIgnored")
161118
@Override
162119
public void close() throws Exception {
@@ -182,58 +139,4 @@ static StorageDataClient create(
182139
return new StorageDataClient(
183140
executor, terminationAwaitDuration, read, retryContextProvider, onClose);
184141
}
185-
186-
@FunctionalInterface
187-
interface Borrowable {
188-
void borrow();
189-
}
190-
191-
static final class FastOpenObjectReadSession<Projection> implements IOAutoCloseable {
192-
private final ObjectReadSession session;
193-
private final ObjectReadSessionStreamRead<Projection> read;
194-
private final Borrowable borrowable;
195-
private boolean sessionLeased;
196-
197-
private FastOpenObjectReadSession(
198-
ObjectReadSession session,
199-
ObjectReadSessionStreamRead<Projection> read,
200-
Borrowable borrowable) {
201-
this.session = session;
202-
this.read = read;
203-
this.borrowable = borrowable;
204-
this.sessionLeased = false;
205-
}
206-
207-
ObjectReadSession getSession() {
208-
if (!sessionLeased) {
209-
sessionLeased = true;
210-
borrowable.borrow();
211-
}
212-
return session;
213-
}
214-
215-
ObjectReadSessionStreamRead<Projection> getRead() {
216-
return read;
217-
}
218-
219-
Projection getProjection() {
220-
return read.project();
221-
}
222-
223-
@Override
224-
public void close() throws IOException {
225-
//noinspection EmptyTryBlock
226-
try (IOAutoCloseable ignore1 = session;
227-
IOAutoCloseable ignore2 = read) {
228-
// use try-with to ensure full cleanup
229-
}
230-
}
231-
232-
public static <Projection> FastOpenObjectReadSession<Projection> of(
233-
ObjectReadSession session,
234-
ObjectReadSessionStreamRead<Projection> read,
235-
Borrowable borrowable) {
236-
return new FastOpenObjectReadSession<>(session, read, borrowable);
237-
}
238-
}
239142
}

‎java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ITObjectReadSessionFakeTest.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import com.google.cloud.storage.Hasher.UncheckedChecksumMismatchException;
4242
import com.google.cloud.storage.OtelStorageDecorator.OtelDecoratingBlobReadSession;
4343
import com.google.cloud.storage.Storage.BlobSourceOption;
44-
import com.google.cloud.storage.StorageDataClient.FastOpenObjectReadSession;
4544
import com.google.cloud.storage.ZeroCopySupport.DisposableByteString;
4645
import com.google.cloud.storage.it.ChecksummedTestContent;
4746
import com.google.cloud.storage.it.GrpcPlainRequestLoggingInterceptor;
@@ -1310,7 +1309,7 @@ public void onCompleted() {
13101309
}
13111310

13121311
@Test
1313-
public void gettingSessionFromFastOpenKeepsTheSessionOpenUntilClosed() throws Exception {
1312+
public void fastOpenRead_thenMoreReadsOnTheSameSession() throws Exception {
13141313
ChecksummedTestContent expected = ChecksummedTestContent.of(ALL_OBJECT_BYTES, 10, 30);
13151314

13161315
ChecksummedTestContent content1 = ChecksummedTestContent.of(ALL_OBJECT_BYTES, 10, 10);
@@ -1384,19 +1383,17 @@ public void gettingSessionFromFastOpenKeepsTheSessionOpenUntilClosed() throws Ex
13841383
.build())
13851384
.build();
13861385

1387-
ApiFuture<FastOpenObjectReadSession<ApiFuture<DisposableByteString>>> future =
1388-
dataClient.fastOpenReadSession(
1389-
req,
1390-
GrpcCallContext.createDefault(),
1391-
ReadProjectionConfigs.asFutureByteString().withRangeSpec(RangeSpec.of(10, 10)));
1386+
ReadAsFutureByteString fastOpenConfig =
1387+
ReadProjectionConfigs.asFutureByteString().withRangeSpec(RangeSpec.of(10, 10));
1388+
ApiFuture<ObjectReadSession> future =
1389+
dataClient.readSession(
1390+
req, GrpcCallContext.createDefault(), ImmutableList.of(fastOpenConfig));
13921391

13931392
ByteString bytes = ByteString.empty();
13941393
Exception caught = null;
13951394

1396-
try (FastOpenObjectReadSession<ApiFuture<DisposableByteString>> fastOpenChannel =
1397-
future.get(5, TimeUnit.SECONDS);
1398-
ObjectReadSession session = fastOpenChannel.getSession()) {
1399-
ApiFuture<DisposableByteString> futureBytes1 = fastOpenChannel.getProjection();
1395+
try (ObjectReadSession session = future.get(5, TimeUnit.SECONDS)) {
1396+
ApiFuture<DisposableByteString> futureBytes1 = session.readAs(fastOpenConfig);
14001397
try (DisposableByteString disposableByteString = futureBytes1.get()) {
14011398
bytes = bytes.concat(disposableByteString.byteString());
14021399
}

‎java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ITStorageDataClientFakeTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
import com.google.api.core.ApiFuture;
2525
import com.google.api.gax.grpc.GrpcCallContext;
2626
import com.google.cloud.storage.ITObjectReadSessionFakeTest.FakeStorage;
27-
import com.google.cloud.storage.StorageDataClient.FastOpenObjectReadSession;
2827
import com.google.cloud.storage.ZeroCopySupport.DisposableByteString;
2928
import com.google.cloud.storage.it.ChecksummedTestContent;
29+
import com.google.common.collect.ImmutableList;
3030
import com.google.common.collect.ImmutableMap;
3131
import com.google.common.io.ByteStreams;
3232
import com.google.protobuf.ByteString;
@@ -137,10 +137,10 @@ private <P> void doTest(ReadProjectionConfig<P> config, ThrowableFunction<P, byt
137137
.build())
138138
.build();
139139

140-
ApiFuture<FastOpenObjectReadSession<P>> f =
141-
dataClient.fastOpenReadSession(req, GrpcCallContext.createDefault(), config);
142-
try (FastOpenObjectReadSession<P> fastOpen = f.get(3, TimeUnit.SECONDS)) {
143-
byte[] apply = func.apply(fastOpen.getProjection());
140+
ApiFuture<ObjectReadSession> f =
141+
dataClient.readSession(req, GrpcCallContext.createDefault(), ImmutableList.of(config));
142+
try (ObjectReadSession session = f.get(3, TimeUnit.SECONDS)) {
143+
byte[] apply = func.apply(session.readAs(config));
144144
assertThat(xxd(apply)).isEqualTo(xxd(content.getBytes()));
145145
}
146146
}

0 commit comments

Comments
 (0)