Skip to content

chore(x-goog-spanner-request-id): add SessionImpl.getChannel() and Options.RequestIdOption.(equals, hashCode) #3900

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 20, 2025
Merged

chore(x-goog-spanner-request-id): add SessionImpl.getChannel() and Options.RequestIdOption.(equals, hashCode) #3900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0c3b8e
chore(x-goog-spanner-request-id): add SessionImpl.getChannel() and Op…
odeke-em Jun 3, 2025
6e65c3e
Return 0 for getChannel() with multiplexed sessions
odeke-em Jun 19, 2025
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
12 changes: 10 additions & 2 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,12 +1088,20 @@ void appendToOptions(Options options) {

@Override
public int hashCode() {
return RequestIdOption.class.hashCode();
return this.reqId.hashCode();
}

@Override
public boolean equals(Object o) {
return o instanceof RequestIdOption;
// instanceof for a null object returns false.
if (!(o instanceof RequestIdOption)) {
return false;
}
RequestIdOption other = (RequestIdOption) o;
if (this.reqId == null || other.reqId == null) {
return this.reqId == null && other.reqId == null;
}
return this.reqId.equals(other.reqId);
Comment on lines +1101 to +1104
Copy link
Collaborator

Choose a reason for hiding this comment

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

tip for next time (just leave as-is for now);Java has a utility method for this:

java.util.Objects.equals(this.reqId, other.reqId);

}
}
}
19 changes: 12 additions & 7 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public CommitResponse writeAtLeastOnceWithOptions(
private XGoogSpannerRequestId reqIdOrFresh(Options options) {
XGoogSpannerRequestId reqId = options.reqId();
if (reqId == null) {
reqId = this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 1);
reqId = this.getRequestIdCreator().nextRequestId(this.getChannel(), 1);
}
return reqId;
}
Expand Down Expand Up @@ -464,17 +464,15 @@ public AsyncTransactionManagerImpl transactionManagerAsync(TransactionOption...

@Override
public ApiFuture<Empty> asyncClose() {
XGoogSpannerRequestId reqId =
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 0);
XGoogSpannerRequestId reqId = this.getRequestIdCreator().nextRequestId(this.getChannel(), 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the initial attempt 0 here and in the close() method, but 1 in beginTransactionAsync()?

return spanner.getRpc().asyncDeleteSession(getName(), reqId.withOptions(getOptions()));
}

@Override
public void close() {
ISpan span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION);
try (IScope s = tracer.withSpan(span)) {
XGoogSpannerRequestId reqId =
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 0);
XGoogSpannerRequestId reqId = this.getRequestIdCreator().nextRequestId(this.getChannel(), 0);
spanner.getRpc().deleteSession(getName(), reqId.withOptions(getOptions()));
} catch (RuntimeException e) {
span.setStatus(e);
Expand Down Expand Up @@ -505,8 +503,7 @@ ApiFuture<Transaction> beginTransactionAsync(
}
final BeginTransactionRequest request = requestBuilder.build();
final ApiFuture<Transaction> requestFuture;
XGoogSpannerRequestId reqId =
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 1);
XGoogSpannerRequestId reqId = this.getRequestIdCreator().nextRequestId(this.getChannel(), 1);
try (IScope ignore = tracer.withSpan(span)) {
requestFuture =
spanner
Expand Down Expand Up @@ -597,4 +594,12 @@ public void setRequestIdCreator(XGoogSpannerRequestId.RequestIdCreator creator)
public XGoogSpannerRequestId.RequestIdCreator getRequestIdCreator() {
return this.requestIdCreator;
}

int getChannel() {
if (getIsMultiplexed()) {
return 0;
}
Comment on lines +599 to +601
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume that this means that we will be getting the channel that is being used in a different way for multiplexed sessions, and that this method is only intended for actual use with regular sessions.

Long channelHint = (Long) this.getOptions().get(SpannerRpc.Option.CHANNEL_HINT);
return (int) (channelHint % this.spanner.getOptions().getNumChannels());
}
}
16 changes: 16 additions & 0 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,22 @@ public void testRequestId() {
assertThat(option3.toString()).doesNotContain("requestId");
}

@Test
public void testRequestIdOptionEqualsAndHashCode() {
XGoogSpannerRequestId reqId1 = XGoogSpannerRequestId.of(1, 2, 3, 4);
XGoogSpannerRequestId reqId2 = XGoogSpannerRequestId.of(2, 3, 4, 5);
Options.RequestIdOption opt1 = Options.requestId(reqId1);
Options.RequestIdOption opt1Prime = Options.requestId(reqId1);
Options.RequestIdOption opt2 = Options.requestId(reqId2);

assertTrue(opt1.equals(opt1));
assertTrue(opt1.equals(opt1Prime));
assertEquals(opt1.hashCode(), opt1Prime.hashCode());
assertFalse(opt1.equals(opt2));
assertNotEquals(opt1, opt2);
assertNotEquals(opt1.hashCode(), opt2.hashCode());
}

@Test
public void testOptions_WithMultipleDifferentRequestIds() {
XGoogSpannerRequestId reqId1 = XGoogSpannerRequestId.of(1, 1, 1, 1);
Expand Down
Loading