Skip to content

Commit 30198ce

Browse files
authored
Merge branch 'main' into refactor-consistency-request
2 parents fd863ae + 8220283 commit 30198ce

13 files changed

Lines changed: 652 additions & 108 deletions

File tree

.github/workflows/hermetic_library_generation.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737
with:
3838
fetch-depth: 0
3939
token: ${{ secrets.CLOUD_JAVA_BOT_TOKEN }}
40-
- uses: googleapis/google-cloud-java/sdk-platform-java/.github/scripts@v1.85.0
40+
- uses: googleapis/google-cloud-java/sdk-platform-java/.github/scripts@google-cloud-shared-dependencies/v3.61.0
4141
if: env.SHOULD_RUN == 'true'
4242
with:
4343
image_tag: latest

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ If you are using Maven without the BOM, add this to your dependencies:
4949
If you are using Gradle 5.x or later, add this to your dependencies:
5050

5151
```Groovy
52-
implementation platform('com.google.cloud:libraries-bom:26.79.0')
52+
implementation platform('com.google.cloud:libraries-bom:26.80.0')
5353
5454
implementation 'com.google.cloud:google-cloud-bigtable'
5555
```

generation_config.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
gapic_generator_version: 2.69.0
2-
googleapis_commitish: 9a832ccde3f3ca4d4e1c39593b1cbf9aa62a75b3
3-
libraries_bom_version: 26.79.0
1+
gapic_generator_version: 2.71.0
2+
googleapis_commitish: 939ba3bf8408af83f0f73ae35c76c4b11a8c8c8d
3+
libraries_bom_version: 26.80.0
44
template_excludes:
55
- .gitignore
66
- .kokoro/presubmit/integration.cfg

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/channels/ChannelPoolDpImpl.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,11 +262,16 @@ private void removeGroup(AfeChannelGroup group) {
262262
}
263263

264264
@GuardedBy("this")
265-
private AfeChannelGroup rehomeChannel(ChannelWrapper channelWrapper, AfeId afeId) {
265+
private void rehomeChannel(ChannelWrapper channelWrapper, AfeId afeId) {
266+
// No need to rehome recycled channels.
267+
if (channelWrapper.channel.isShutdown()) {
268+
return;
269+
}
270+
266271
AfeChannelGroup origGroup = channelWrapper.group;
267272

268273
if (Objects.equals(origGroup.afeId, afeId)) {
269-
return origGroup;
274+
return;
270275
}
271276

272277
log(Level.FINE, "Rehoming channel from: %s to %s", origGroup.afeId, afeId);
@@ -291,7 +296,7 @@ private AfeChannelGroup rehomeChannel(ChannelWrapper channelWrapper, AfeId afeId
291296
newGroup.channels.add(channelWrapper);
292297
newGroup.numStreams += channelWrapper.numOutstanding;
293298

294-
return newGroup;
299+
return;
295300
}
296301

297302
// Update accounting when a stream is closed and releases its channel
@@ -322,6 +327,11 @@ private static boolean shouldRecycleChannel(Status status) {
322327

323328
@GuardedBy("this")
324329
private void recycleChannel(ChannelWrapper channelWrapper) {
330+
if (channelWrapper.channel.isShutdown()) {
331+
// Channel is already recycled.
332+
return;
333+
}
334+
325335
channelWrapper.group.channels.remove(channelWrapper);
326336
channelWrapper.channel.shutdown();
327337
// Checking for starting group because we don't want to delete the stating group.

google-cloud-bigtable/src/main/resources/META-INF/native-image/com.google.cloud.bigtable.admin.v2/reflect-config.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1871,6 +1871,15 @@
18711871
"allDeclaredClasses": true,
18721872
"allPublicClasses": true
18731873
},
1874+
{
1875+
"name": "com.google.bigtable.admin.v2.Instance$Edition",
1876+
"queryAllDeclaredConstructors": true,
1877+
"queryAllPublicConstructors": true,
1878+
"queryAllDeclaredMethods": true,
1879+
"allPublicMethods": true,
1880+
"allDeclaredClasses": true,
1881+
"allPublicClasses": true
1882+
},
18741883
{
18751884
"name": "com.google.bigtable.admin.v2.Instance$State",
18761885
"queryAllDeclaredConstructors": true,

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/admin/v2/BaseBigtableInstanceAdminClientTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ public void updateInstanceTest() throws Exception {
475475
Assert.assertEquals(request.getDisplayName(), actualRequest.getDisplayName());
476476
Assert.assertEquals(request.getState(), actualRequest.getState());
477477
Assert.assertEquals(request.getType(), actualRequest.getType());
478+
Assert.assertEquals(request.getEdition(), actualRequest.getEdition());
478479
Assert.assertEquals(request.getLabelsMap(), actualRequest.getLabelsMap());
479480
Assert.assertEquals(request.getCreateTime(), actualRequest.getCreateTime());
480481
Assert.assertEquals(request.getSatisfiesPzs(), actualRequest.getSatisfiesPzs());

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/channels/ChannelPoolDpImplTest.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,4 +375,98 @@ void testRecycleChannelInGroupOnUnimplemented() {
375375

376376
pool.close();
377377
}
378+
379+
@Test
380+
void testDoubleRecycleCreatesExtraChannel() {
381+
when(channelSupplier.get()).thenReturn(channel);
382+
when(channel.newCall(any(), any())).thenReturn(clientCall);
383+
doNothing().when(clientCall).start(listener.capture(), any());
384+
385+
ChannelPoolDpImpl pool =
386+
new ChannelPoolDpImpl(channelSupplier, defaultConfig, debugTagTracer, bgExecutor);
387+
388+
// Create 2 streams on the same channel
389+
pool.newStream(FakeSessionGrpc.getOpenSessionMethod(), CallOptions.DEFAULT)
390+
.start(Mockito.mock(Listener.class), new Metadata());
391+
pool.newStream(FakeSessionGrpc.getOpenSessionMethod(), CallOptions.DEFAULT)
392+
.start(Mockito.mock(Listener.class), new Metadata());
393+
394+
// Initially 1 channel
395+
verify(channelSupplier, times(1)).get();
396+
397+
// Trigger first recycle via first stream
398+
ClientCall.Listener<Object> listener1 = listener.getAllValues().get(0);
399+
listener1.onClose(Status.UNIMPLEMENTED, new Metadata());
400+
401+
// Channel should be recycled (shutdown + addChannel)
402+
verify(channel, times(1)).shutdown();
403+
// Now isShutdown returns true for the channel
404+
when(channel.isShutdown()).thenReturn(true);
405+
verify(channelSupplier, times(2)).get();
406+
407+
// Trigger second recycle via second stream on the SAME (already recycled) channel
408+
ClientCall.Listener<Object> listener2 = listener.getAllValues().get(1);
409+
listener2.onClose(Status.UNIMPLEMENTED, new Metadata());
410+
411+
// BUG: This should NOT cause another addChannel() or shutdown()
412+
// If it fails, times(3) will be true.
413+
verify(channelSupplier, times(2)).get();
414+
verify(channel, times(1)).shutdown();
415+
416+
pool.close();
417+
}
418+
419+
@Test
420+
void testRecycledChannelDoesNotRejoinPool() throws InterruptedException {
421+
when(channelSupplier.get()).thenReturn(channel);
422+
when(channel.newCall(any(), any())).thenReturn(clientCall);
423+
doNothing().when(clientCall).start(listener.capture(), any());
424+
doReturn(Attributes.EMPTY).when(clientCall).getAttributes();
425+
426+
ChannelPoolDpImpl pool =
427+
new ChannelPoolDpImpl(channelSupplier, defaultConfig, debugTagTracer, bgExecutor);
428+
429+
// 1. Create stream1 on channel1
430+
pool.newStream(FakeSessionGrpc.getOpenSessionMethod(), CallOptions.DEFAULT)
431+
.start(Mockito.mock(Listener.class), new Metadata());
432+
433+
// 2. Create stream2 on channel1 to trigger recycle
434+
pool.newStream(FakeSessionGrpc.getOpenSessionMethod(), CallOptions.DEFAULT)
435+
.start(Mockito.mock(Listener.class), new Metadata());
436+
437+
ClientCall.Listener<Object> listener1 = listener.getAllValues().get(0);
438+
ClientCall.Listener<Object> listener2 = listener.getAllValues().get(1);
439+
440+
// Prepare channel2 that will be picked up by addChannel during recycling
441+
ManagedChannel channel2 = Mockito.mock(ManagedChannel.class);
442+
when(channelSupplier.get()).thenReturn(channel2);
443+
when(channel2.newCall(any(), any())).thenReturn(clientCall);
444+
445+
// 3. Recycle channel1 via stream2
446+
listener2.onClose(Status.UNIMPLEMENTED, new Metadata());
447+
verify(channel, times(1)).shutdown();
448+
// Now isShutdown for the channel1 returns true
449+
when(channel.isShutdown()).thenReturn(true);
450+
451+
// 4. stream1 (on recycled channel1) receives headers with AFE ID
452+
// This triggers rehomeChannel
453+
PeerInfo peerInfo = PeerInfo.newBuilder().setApplicationFrontendId(555).build();
454+
Metadata headers = new Metadata();
455+
headers.put(
456+
SessionStreamImpl.PEER_INFO_KEY,
457+
Base64.getEncoder().encodeToString(peerInfo.toByteArray()));
458+
listener1.onHeaders(headers);
459+
460+
// 5. Try to create a new stream.
461+
// It should NOT pick channel1 because it's recycled/shutdown.
462+
pool.newStream(FakeSessionGrpc.getOpenSessionMethod(), CallOptions.DEFAULT);
463+
464+
// BUG: If channel1 was re-added to a group, the picker might have picked it.
465+
// channel.newCall was called 2 times (steps 1 and 2). It should NOT be called a 3rd time.
466+
verify(channel, times(2)).newCall(any(), any());
467+
// Instead, it should be called on channel2
468+
verify(channel2, times(1)).newCall(any(), any());
469+
470+
pool.close();
471+
}
378472
}

proto-google-cloud-bigtable-admin-v2/src/main/java/com/google/bigtable/admin/v2/AppProfile.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4510,7 +4510,7 @@ public com.google.bigtable.admin.v2.AppProfile.SingleClusterRouting getSingleClu
45104510
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
45114511
*
45124512
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
4513-
* google/bigtable/admin/v2/instance.proto;l=421
4513+
* google/bigtable/admin/v2/instance.proto;l=448
45144514
* @return Whether the priority field is set.
45154515
*/
45164516
@java.lang.Deprecated
@@ -4531,7 +4531,7 @@ public boolean hasPriority() {
45314531
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
45324532
*
45334533
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
4534-
* google/bigtable/admin/v2/instance.proto;l=421
4534+
* google/bigtable/admin/v2/instance.proto;l=448
45354535
* @return The enum numeric value on the wire for priority.
45364536
*/
45374537
@java.lang.Deprecated
@@ -4555,7 +4555,7 @@ public int getPriorityValue() {
45554555
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
45564556
*
45574557
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
4558-
* google/bigtable/admin/v2/instance.proto;l=421
4558+
* google/bigtable/admin/v2/instance.proto;l=448
45594559
* @return The priority.
45604560
*/
45614561
@java.lang.Deprecated
@@ -6158,7 +6158,7 @@ public Builder clearSingleClusterRouting() {
61586158
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
61596159
*
61606160
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
6161-
* google/bigtable/admin/v2/instance.proto;l=421
6161+
* google/bigtable/admin/v2/instance.proto;l=448
61626162
* @return Whether the priority field is set.
61636163
*/
61646164
@java.lang.Override
@@ -6180,7 +6180,7 @@ public boolean hasPriority() {
61806180
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
61816181
*
61826182
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
6183-
* google/bigtable/admin/v2/instance.proto;l=421
6183+
* google/bigtable/admin/v2/instance.proto;l=448
61846184
* @return The enum numeric value on the wire for priority.
61856185
*/
61866186
@java.lang.Override
@@ -6205,7 +6205,7 @@ public int getPriorityValue() {
62056205
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
62066206
*
62076207
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
6208-
* google/bigtable/admin/v2/instance.proto;l=421
6208+
* google/bigtable/admin/v2/instance.proto;l=448
62096209
* @param value The enum numeric value on the wire for priority to set.
62106210
* @return This builder for chaining.
62116211
*/
@@ -6230,7 +6230,7 @@ public Builder setPriorityValue(int value) {
62306230
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
62316231
*
62326232
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
6233-
* google/bigtable/admin/v2/instance.proto;l=421
6233+
* google/bigtable/admin/v2/instance.proto;l=448
62346234
* @return The priority.
62356235
*/
62366236
@java.lang.Override
@@ -6260,7 +6260,7 @@ public com.google.bigtable.admin.v2.AppProfile.Priority getPriority() {
62606260
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
62616261
*
62626262
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
6263-
* google/bigtable/admin/v2/instance.proto;l=421
6263+
* google/bigtable/admin/v2/instance.proto;l=448
62646264
* @param value The priority to set.
62656265
* @return This builder for chaining.
62666266
*/
@@ -6288,7 +6288,7 @@ public Builder setPriority(com.google.bigtable.admin.v2.AppProfile.Priority valu
62886288
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
62896289
*
62906290
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
6291-
* google/bigtable/admin/v2/instance.proto;l=421
6291+
* google/bigtable/admin/v2/instance.proto;l=448
62926292
* @return This builder for chaining.
62936293
*/
62946294
@java.lang.Deprecated

proto-google-cloud-bigtable-admin-v2/src/main/java/com/google/bigtable/admin/v2/AppProfileOrBuilder.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ public interface AppProfileOrBuilder
218218
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
219219
*
220220
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
221-
* google/bigtable/admin/v2/instance.proto;l=421
221+
* google/bigtable/admin/v2/instance.proto;l=448
222222
* @return Whether the priority field is set.
223223
*/
224224
@java.lang.Deprecated
@@ -237,7 +237,7 @@ public interface AppProfileOrBuilder
237237
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
238238
*
239239
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
240-
* google/bigtable/admin/v2/instance.proto;l=421
240+
* google/bigtable/admin/v2/instance.proto;l=448
241241
* @return The enum numeric value on the wire for priority.
242242
*/
243243
@java.lang.Deprecated
@@ -256,7 +256,7 @@ public interface AppProfileOrBuilder
256256
* <code>.google.bigtable.admin.v2.AppProfile.Priority priority = 7 [deprecated = true];</code>
257257
*
258258
* @deprecated google.bigtable.admin.v2.AppProfile.priority is deprecated. See
259-
* google/bigtable/admin/v2/instance.proto;l=421
259+
* google/bigtable/admin/v2/instance.proto;l=448
260260
* @return The priority.
261261
*/
262262
@java.lang.Deprecated

0 commit comments

Comments
 (0)