From e6233c516ccd6812bc5e56357665f0d3dd18d5e4 Mon Sep 17 00:00:00 2001 From: Bradley White <14679271+devbww@users.noreply.github.com> Date: Wed, 17 Apr 2024 12:06:52 -0400 Subject: [PATCH] fix(spanner): delete idle sessions upon SessionPool destruction Send a fire-and-forget `AsyncDeleteSession()` request for all idle sessions during `~SessionPool()`. Previously they would have continued to exist in the service until garbage collected due to inactivity after 60 minutes. Add expectations for these requests in the connection/session_pool unit tests. --- .../spanner/internal/connection_impl_test.cc | 271 ++++++++++++++++++ google/cloud/spanner/internal/session_pool.cc | 7 + .../spanner/internal/session_pool_test.cc | 92 ++++++ 3 files changed, 370 insertions(+) diff --git a/google/cloud/spanner/internal/connection_impl_test.cc b/google/cloud/spanner/internal/connection_impl_test.cc index 931e83f5b4ef7..80008f5cacabe 100644 --- a/google/cloud/spanner/internal/connection_impl_test.cc +++ b/google/cloud/spanner/internal/connection_impl_test.cc @@ -170,6 +170,10 @@ MATCHER_P(HasCreatorRole, role, "has creator role") { return arg.session_template().creator_role() == role; } +MATCHER_P(HasSessionName, name, "has session name") { + return arg.name() == name; +} + // Matches a `spanner::Transaction` that is bound to a "bad" session. MATCHER(HasBadSession, "is bound to a session that's marked bad") { return Visit(arg, [&](SessionHolder& session, @@ -363,6 +367,7 @@ TEST(ConnectionImplTest, ReadCreateSessionFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -388,6 +393,9 @@ TEST(ConnectionImplTest, ReadStreamingReadFailure) { EXPECT_CALL(*mock, StreamingRead) .WillOnce( Return(ByMove(MakeReader({}, finish_status)))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -470,6 +478,9 @@ TEST(ConnectionImplTest, ReadSuccess) { EXPECT_THAT(request.resume_token(), Eq("restart-row-2")); return MakeReader({responses[1], responses[2]}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -512,6 +523,9 @@ TEST(ConnectionImplTest, ReadDirectedRead) { return MakeReader( {R"pb(metadata: { transaction: { id: "ABCDEF00" } })pb"}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -543,6 +557,9 @@ TEST(ConnectionImplTest, ReadPermanentFailure) { EXPECT_CALL(*mock, StreamingRead) .WillOnce(Return(ByMove(MakeReader( {}, internal::PermissionDeniedError("uh-oh"))))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -570,6 +587,9 @@ TEST(ConnectionImplTest, ReadTooManyTransientFailures) { return MakeReader( {}, internal::UnavailableError("try-again")); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -597,6 +617,9 @@ TEST(ConnectionImplTest, ReadImplicitBeginTransaction) { )pb"; EXPECT_CALL(*mock, StreamingRead) .WillOnce(Return(ByMove(MakeReader({kText})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -652,6 +675,10 @@ TEST(ConnectionImplTest, ReadImplicitBeginTransactionOneTransientFailure) { HasBeginTransaction()))) .InSequence(s) .WillOnce(Return(ByMove(std::move(ok_reader)))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .InSequence(s) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -714,6 +741,10 @@ TEST(ConnectionImplTest, ReadImplicitBeginTransactionOnePermanentFailure) { HasTransactionId("FEDCBA98")))) .InSequence(s) .WillOnce(Return(ByMove(std::move(ok_reader)))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .InSequence(s) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -758,6 +789,10 @@ TEST(ConnectionImplTest, ReadImplicitBeginTransactionPermanentFailure) { HasTransactionId("FEDCBA98")))) .InSequence(s) .WillOnce(Return(ByMove(std::move(reader2)))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .InSequence(s) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -779,6 +814,7 @@ TEST(ConnectionImplTest, ExecuteQueryCreateSessionFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -801,6 +837,9 @@ TEST(ConnectionImplTest, ExecuteQueryStreamingReadFailure) { .WillOnce(Return(ByMove(MakeReader( {}, internal::PermissionDeniedError("uh-oh in GrpcReader::Finish"))))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -845,6 +884,9 @@ TEST(ConnectionImplTest, ExecuteQueryReadSuccess) { )pb"; EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce(Return(ByMove(MakeReader({kText})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -886,6 +928,9 @@ TEST(ConnectionImplTest, ExecuteQueryDirectedRead) { return MakeReader( {R"pb(metadata: { transaction: { id: "00FEDCBA" } })pb"}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -930,6 +975,9 @@ TEST(ConnectionImplTest, ExecuteQueryPgNumericResult) { )pb"; EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce(Return(ByMove(MakeReader({kText})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -976,6 +1024,9 @@ TEST(ConnectionImplTest, ExecuteQueryJsonBResult) { )pb"; EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce(Return(ByMove(MakeReader({kText})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1051,6 +1102,9 @@ TEST(ConnectionImplTest, ExecuteQueryNumericParameter) { google::spanner::v1::TypeAnnotationCode::PG_NUMERIC); return MakeReader({kResponsePgNumeric}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1103,6 +1157,9 @@ TEST(ConnectionImplTest, ExecuteQueryPgOidResult) { )pb"; EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce(Return(ByMove(MakeReader({kText})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1133,6 +1190,9 @@ TEST(ConnectionImplTest, ExecuteQueryImplicitBeginTransaction) { )pb"; EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce(Return(ByMove(MakeReader({kText})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1391,6 +1451,11 @@ TEST(ConnectionImplTest, QueryOptions) { EXPECT_CALL(*mock, Commit(_, _, commit_request_matcher)) .Times(1) .RetiresOnSaturation(); + + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))) + .RetiresOnSaturation(); } auto conn = MakeConnectionImpl(db, mock); @@ -1415,6 +1480,7 @@ TEST(ConnectionImplTest, ExecuteDmlCreateSessionFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1442,6 +1508,9 @@ TEST(ConnectionImplTest, ExecuteDmlDeleteSuccess) { EXPECT_CALL(*mock, ExecuteSql) .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(response)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1467,6 +1536,9 @@ TEST(ConnectionImplTest, ExecuteDmlDeletePermanentFailure) { EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(MakeTestTransaction())); EXPECT_CALL(*mock, ExecuteSql).WillOnce(Return(status)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto conn = MakeConnectionImpl(db, mock); @@ -1496,6 +1568,9 @@ TEST(ConnectionImplTest, ExecuteDmlDeleteTooManyTransientFailures) { EXPECT_CALL(*mock, ExecuteSql) .Times(AtLeast(2)) .WillRepeatedly(Return(status)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto conn = MakeConnectionImpl(db, mock); @@ -1527,6 +1602,10 @@ TEST(ConnectionImplTest, ExecuteDmlTransactionAtomicity) { // not valid the client fails any subsequent operations itself. EXPECT_CALL(*mock, ExecuteSql).WillOnce(Return(op_status)); EXPECT_CALL(*mock, BeginTransaction).WillOnce(Return(begin_status)); + + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto conn = MakeConnectionImpl(db, mock); @@ -1557,6 +1636,10 @@ TEST(ConnectionImplTest, ExecuteDmlTransactionMissing) { ASSERT_TRUE(TextFormat::ParseFromString("metadata: {}", &response)); EXPECT_CALL(*mock, ExecuteSql).WillOnce(Return(response)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); + auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); spanner::Transaction txn = @@ -1603,6 +1686,9 @@ TEST(ConnectionImplTest, ProfileQuerySuccess) { )pb"; EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce(Return(ByMove(MakeReader({kText})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1640,6 +1726,7 @@ TEST(ConnectionImplTest, ProfileQueryCreateSessionFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1663,6 +1750,9 @@ TEST(ConnectionImplTest, ProfileQueryStreamingReadFailure) { EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce( Return(ByMove(MakeReader({}, finish_status)))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1683,6 +1773,7 @@ TEST(ConnectionImplTest, ProfileDmlCreateSessionFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1718,6 +1809,9 @@ TEST(ConnectionImplTest, ProfileDmlDeleteSuccess) { EXPECT_CALL(*mock, ExecuteSql) .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(response)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1758,6 +1852,9 @@ TEST(ConnectionImplTest, ProfileDmlDeletePermanentFailure) { EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(MakeTestTransaction())); EXPECT_CALL(*mock, ExecuteSql).WillOnce(Return(status)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto conn = MakeConnectionImpl(db, mock); @@ -1787,6 +1884,9 @@ TEST(ConnectionImplTest, ProfileDmlDeleteTooManyTransientFailures) { EXPECT_CALL(*mock, ExecuteSql) .Times(AtLeast(2)) .WillRepeatedly(Return(status)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto conn = MakeConnectionImpl(db, mock); @@ -1815,6 +1915,9 @@ TEST(ConnectionImplTest, AnalyzeSqlSuccess) { EXPECT_CALL(*mock, ExecuteSql) .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(ByMove(std::move(response)))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1839,6 +1942,7 @@ TEST(ConnectionImplTest, AnalyzeSqlCreateSessionFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -1863,6 +1967,9 @@ TEST(ConnectionImplTest, AnalyzeSqlDeletePermanentFailure) { EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(MakeTestTransaction())); EXPECT_CALL(*mock, ExecuteSql).WillOnce(Return(status)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto conn = MakeConnectionImpl(db, mock); @@ -1892,6 +1999,9 @@ TEST(ConnectionImplTest, AnalyzeSqlDeleteTooManyTransientFailures) { EXPECT_CALL(*mock, ExecuteSql) .Times(AtLeast(2)) .WillRepeatedly(Return(status)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto conn = MakeConnectionImpl(db, mock); @@ -1927,6 +2037,9 @@ TEST(ConnectionImplTest, ExecuteBatchDmlSuccess) { HasPriority(google::spanner::v1::RequestOptions::PRIORITY_MEDIUM))) .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(response)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto request = { spanner::SqlStatement("UPDATE ..."), @@ -1970,6 +2083,9 @@ TEST(ConnectionImplTest, ExecuteBatchDmlPartialFailure) { google::spanner::v1::ExecuteBatchDmlResponse response; ASSERT_TRUE(TextFormat::ParseFromString(kText, &response)); EXPECT_CALL(*mock, ExecuteBatchDml).WillOnce(Return(response)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto request = { spanner::SqlStatement("UPDATE ..."), @@ -2006,6 +2122,9 @@ TEST(ConnectionImplTest, ExecuteBatchDmlPermanentFailure) { EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(MakeTestTransaction())); EXPECT_CALL(*mock, ExecuteBatchDml).WillOnce(Return(status)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto request = { @@ -2040,6 +2159,10 @@ TEST(ConnectionImplTest, ExecuteBatchDmlTooManyTransientFailures) { EXPECT_CALL(*mock, ExecuteBatchDml) .Times(AtLeast(2)) .WillRepeatedly(Return(status)); + + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto request = { @@ -2081,6 +2204,9 @@ TEST(ConnectionImplTest, ExecuteBatchDmlNoResultSets) { AllOf(HasSession("session-name"), HasTransactionId("BD000001")))) .WillOnce(Return(response)); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); } auto request = {spanner::SqlStatement("UPDATE ...")}; @@ -2120,6 +2246,9 @@ TEST(ConnectionImplTest, ExecutePartitionedDmlDeleteSuccess) { {}, internal::UnavailableError("try-again in ExecutePartitionedDml"))))) .WillOnce(Return(ByMove(MakeReader({kTextResponse})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2152,6 +2281,9 @@ TEST(ConnectionImplTest, ExecutePartitionedDmlExcludeFromChangeStreams) { ExecuteStreamingSql( _, _, AllOf(HasRequestTag("tag"), HasTransactionTag("")))) .WillOnce(Return(ByMove(MakeReader({kTextResponse})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span( @@ -2172,6 +2304,7 @@ TEST(ConnectionImplTest, ExecutePartitionedDmlCreateSessionFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2197,6 +2330,10 @@ TEST(ConnectionImplTest, ExecutePartitionedDmlDeletePermanentFailure) { .WillOnce(Return(ByMove(MakeReader( {}, internal::InternalError("permanent failure"))))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); + auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); auto result = conn->ExecutePartitionedDml( @@ -2222,6 +2359,9 @@ TEST(ConnectionImplTest, ExecutePartitionedDmlDeleteTooManyTransientFailures) { {}, internal::UnavailableError("try-again in ExecutePartitionedDml")); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2257,6 +2397,10 @@ TEST(ConnectionImplTest, ExecutePartitionedDmlRetryableInternalErrors) { "HTTP/2 error code: INTERNAL_ERROR\nReceived Rst Stream"))))) .WillOnce(Return(ByMove(MakeReader({kTextResponse})))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); + auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); auto result = conn->ExecutePartitionedDml( @@ -2275,6 +2419,9 @@ TEST(ConnectionImplTest, EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(Status(StatusCode::kPermissionDenied, "uh-oh in ExecutePartitionedDml"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2295,6 +2442,9 @@ TEST(ConnectionImplTest, .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kUnavailable, "try-again in ExecutePartitionedDml"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2313,6 +2463,7 @@ TEST(ConnectionImplTest, CommitCreateSessionPermanentFailure) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2329,6 +2480,7 @@ TEST(ConnectionImplTest, CommitCreateSessionTooManyTransientFailures) { .Times(AtLeast(2)) .WillRepeatedly(Return(Status(StatusCode::kUnavailable, "try-again in BatchCreateSessions"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2352,6 +2504,9 @@ TEST(ConnectionImplTest, CommitCreateSessionRetry) { HasNakedTransactionId(txn.id())))) .WillOnce( Return(Status(StatusCode::kPermissionDenied, "uh-oh in Commit"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2377,6 +2532,9 @@ TEST(ConnectionImplTest, CommitBeginTransactionRetry) { AllOf(HasSession("test-session-name"), HasNakedTransactionId(txn.id())))) .WillOnce(Return(MakeCommitResponse(commit_timestamp))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2393,6 +2551,7 @@ TEST(ConnectionImplTest, CommitBeginTransactionSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2413,6 +2572,9 @@ TEST(ConnectionImplTest, CommitBeginTransactionPermanentFailure) { EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return( Status(StatusCode::kInvalidArgument, "BeginTransaction failed"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2441,6 +2603,9 @@ TEST(ConnectionImplTest, CommitCommitPermanentFailure) { HasNakedTransactionId(txn.id())))) .WillOnce( Return(Status(StatusCode::kPermissionDenied, "uh-oh in Commit"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2462,6 +2627,9 @@ TEST(ConnectionImplTest, CommitCommitTooManyTransientFailures) { HasNakedTransactionId(txn.id())))) .WillOnce( Return(Status(StatusCode::kPermissionDenied, "uh-oh in Commit"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2478,6 +2646,9 @@ TEST(ConnectionImplTest, CommitCommitInvalidatedTransaction) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, BeginTransaction).Times(0); EXPECT_CALL(*mock, Commit).Times(0); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2506,6 +2677,9 @@ TEST(ConnectionImplTest, CommitCommitIdempotentTransientSuccess) { HasNakedTransactionId("test-txn-id")))) .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(MakeCommitResponse(commit_timestamp))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2535,6 +2709,9 @@ TEST(ConnectionImplTest, CommitSuccessWithTransactionId) { .WillOnce(Return(MakeCommitResponse( spanner::MakeTimestamp(std::chrono::system_clock::from_time_t(123)) .value()))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2571,6 +2748,9 @@ TEST(ConnectionImplTest, CommitSuccessWithStats) { spanner::MakeTimestamp(std::chrono::system_clock::from_time_t(123)) .value(), spanner::CommitStats{42}))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2600,6 +2780,9 @@ TEST(ConnectionImplTest, CommitSuccessExcludeFromChangeStreams) { .WillOnce(Return(MakeCommitResponse( spanner::MakeTimestamp(std::chrono::system_clock::from_time_t(123)) .value()))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span( @@ -2628,6 +2811,9 @@ TEST(ConnectionImplTest, CommitSuccessWithMaxCommitDelay) { .WillOnce(Return(MakeCommitResponse( spanner::MakeTimestamp(std::chrono::system_clock::from_time_t(123)) .value()))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2654,6 +2840,9 @@ TEST(ConnectionImplTest, CommitSuccessWithCompression) { spanner::MakeTimestamp(std::chrono::system_clock::from_time_t(123)) .value()); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span( @@ -2691,6 +2880,9 @@ TEST(ConnectionImplTest, CommitAtLeastOnce) { EXPECT_THAT(request.request_options().transaction_tag(), IsEmpty()); return MakeCommitResponse(commit_timestamp); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2745,6 +2937,9 @@ TEST(ConnectionImplTest, CommitAtLeastOnceBatched) { commit_timestamp { seconds: 123 } )pb"}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2786,6 +2981,9 @@ TEST(ConnectionImplTest, CommitAtLeastOnceBatchedSessionNotFound) { {R"pb(status {} commit_timestamp { seconds: 123 })pb"}); }); + EXPECT_CALL( + *mock, AsyncDeleteSession(_, _, _, HasSessionName("test-session-name-2"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2844,6 +3042,9 @@ TEST(ConnectionImplTest, CommitAtLeastOnceBatchedExcludeFromChangeStreams) { commit_timestamp { seconds: 123 } )pb"}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span( @@ -2869,6 +3070,7 @@ TEST(ConnectionImplTest, RollbackCreateSessionFailure) { .WillRepeatedly(Return(Status(StatusCode::kPermissionDenied, "uh-oh in BatchCreateSessions"))); EXPECT_CALL(*mock, Rollback).Times(0); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2892,6 +3094,9 @@ TEST(ConnectionImplTest, RollbackBeginTransaction) { AllOf(HasSession(session_name), HasNakedTransactionId(transaction_id)))) .WillOnce(Return(Status())); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2906,6 +3111,9 @@ TEST(ConnectionImplTest, RollbackSingleUseTransaction) { EXPECT_CALL(*mock, BatchCreateSessions(_, _, HasDatabase(db))) .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, Rollback).Times(0); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2929,6 +3137,9 @@ TEST(ConnectionImplTest, RollbackPermanentFailure) { HasNakedTransactionId(transaction_id)))) .WillOnce( Return(Status(StatusCode::kPermissionDenied, "uh-oh in Rollback"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2952,6 +3163,9 @@ TEST(ConnectionImplTest, RollbackTooManyTransientFailures) { .Times(AtLeast(2)) .WillRepeatedly( Return(Status(StatusCode::kUnavailable, "try-again in Rollback"))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -2974,6 +3188,9 @@ TEST(ConnectionImplTest, RollbackSuccess) { HasNakedTransactionId(transaction_id)))) .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(Status())); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -2990,6 +3207,9 @@ TEST(ConnectionImplTest, RollbackInvalidatedTransaction) { EXPECT_CALL(*mock, BatchCreateSessions(_, _, HasDatabase(db))) .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, Rollback).Times(0); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -3022,6 +3242,9 @@ TEST(ConnectionImplTest, ReadPartition) { return MakeReader( {R"pb(metadata: { transaction: { id: "ABCDEF00" } })pb"}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -3070,6 +3293,8 @@ TEST(ConnectionImplTest, PartitionReadSuccess) { .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(partition_response)); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); // disassociated + auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); spanner::Transaction txn = @@ -3116,6 +3341,7 @@ TEST(ConnectionImplTest, PartitionReadPermanentFailure) { EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(MakeTestTransaction())); EXPECT_CALL(*mock, PartitionRead).WillOnce(Return(status)); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); // disassociated } auto conn = MakeConnectionImpl(db, mock); @@ -3146,6 +3372,7 @@ TEST(ConnectionImplTest, PartitionReadTooManyTransientFailures) { EXPECT_CALL(*mock, PartitionRead) .Times(AtLeast(2)) .WillRepeatedly(Return(status)); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); // disassociated } auto conn = MakeConnectionImpl(db, mock); @@ -3177,6 +3404,9 @@ TEST(ConnectionImplTest, QueryPartition) { return MakeReader( {R"pb(metadata: { transaction: { id: "ABCDEF00" } })pb"}); }); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -3219,6 +3449,8 @@ TEST(ConnectionImplTest, PartitionQuerySuccess) { .WillOnce(Return(Status(StatusCode::kUnavailable, "try-again"))) .WillOnce(Return(partition_response)); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); // disassociated + auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); spanner::SqlStatement sql_statement("SELECT * FROM Table"); @@ -3253,6 +3485,7 @@ TEST(ConnectionImplTest, PartitionQueryPermanentFailure) { EXPECT_CALL(*mock, BeginTransaction) .WillOnce(Return(MakeTestTransaction())); EXPECT_CALL(*mock, PartitionQuery).WillOnce(Return(failed_status)); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); // disassociated } auto conn = MakeConnectionImpl(db, mock); @@ -3283,6 +3516,7 @@ TEST(ConnectionImplTest, PartitionQueryTooManyTransientFailures) { EXPECT_CALL(*mock, PartitionQuery) .Times(AtLeast(2)) .WillRepeatedly(Return(failed_status)); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); // disassociated } auto conn = MakeConnectionImpl(db, mock); @@ -3321,6 +3555,15 @@ TEST(ConnectionImplTest, MultipleThreads) { EXPECT_THAT(request.session(), StartsWith(session_prefix)); return Status(); }); + EXPECT_CALL(*mock, AsyncDeleteSession) + .WillRepeatedly( + [session_prefix]( + CompletionQueue&, std::shared_ptr const&, + internal::ImmutableOptions const&, + google::spanner::v1::DeleteSessionRequest const& request) { + EXPECT_THAT(request.name(), StartsWith(session_prefix)); + return make_ready_future(Status{}); + }); int const per_thread_iterations = 1000; auto const thread_count = []() -> unsigned { @@ -3419,6 +3662,11 @@ TEST(ConnectionImplTest, TransactionSessionBinding) { .WillOnce(Return(ByMove(std::move(readers[3])))); } + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, HasSessionName("session-1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, HasSessionName("session-2"))) + .WillOnce(Return(make_ready_future(Status{}))); + // Now do the actual reads and verify the results. auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); @@ -3479,6 +3727,10 @@ TEST(ConnectionImplTest, TransactionOutlivesConnection) { EXPECT_CALL(*mock, StreamingRead) .WillOnce(Return(ByMove(MakeReader({kText})))); + // Because the transaction outlives the connection, the session is not in + // the pool when the connection is destroyed, so the session is leaked. + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); + auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); spanner::Transaction txn = @@ -3529,6 +3781,12 @@ TEST(ConnectionImplTest, ReadSessionNotFound) { } )pb"}); }); + EXPECT_CALL( + *mock, AsyncDeleteSession(_, _, _, HasSessionName("test-session-name-1"))) + .Times(0); + EXPECT_CALL( + *mock, AsyncDeleteSession(_, _, _, HasSessionName("test-session-name-2"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3565,6 +3823,7 @@ TEST(ConnectionImplTest, PartitionReadSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, PartitionRead) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3587,6 +3846,7 @@ TEST(ConnectionImplTest, ExecuteQuerySessionNotFound) { EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce( Return(ByMove(MakeReader({}, finish_status)))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3608,6 +3868,7 @@ TEST(ConnectionImplTest, ProfileQuerySessionNotFound) { EXPECT_CALL(*mock, ExecuteStreamingSql) .WillOnce( Return(ByMove(MakeReader({}, finish_status)))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3627,6 +3888,7 @@ TEST(ConnectionImplTest, ExecuteDmlSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, ExecuteSql) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3646,6 +3908,7 @@ TEST(ConnectionImplTest, ProfileDmlSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, ExecuteSql) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3665,6 +3928,7 @@ TEST(ConnectionImplTest, AnalyzeSqlSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, ExecuteSql) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3684,6 +3948,7 @@ TEST(ConnectionImplTest, PartitionQuerySessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, PartitionQuery) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3703,6 +3968,7 @@ TEST(ConnectionImplTest, ExecuteBatchDmlSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, ExecuteBatchDml) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3729,6 +3995,7 @@ TEST(ConnectionImplTest, CommitSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, Commit) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3748,6 +4015,7 @@ TEST(ConnectionImplTest, RollbackSessionNotFound) { .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); EXPECT_CALL(*mock, Rollback) .WillOnce(Return(SessionNotFoundError("test-session-name"))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedRetryOptions()); @@ -3764,6 +4032,9 @@ TEST(ConnectionImplTest, OperationsFailOnInvalidatedTransaction) { "placeholder_database_id"); EXPECT_CALL(*mock, BatchCreateSessions(_, _, HasDatabase(db))) .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); auto conn = MakeConnectionImpl(db, mock); internal::OptionsSpan span(MakeLimitedTimeOptions()); diff --git a/google/cloud/spanner/internal/session_pool.cc b/google/cloud/spanner/internal/session_pool.cc index 8481d79e33cc7..6f20028a3c787 100644 --- a/google/cloud/spanner/internal/session_pool.cc +++ b/google/cloud/spanner/internal/session_pool.cc @@ -97,6 +97,13 @@ SessionPool::~SessionPool() { // `weak_ptr` to `this` they hold. Any in-progress or subsequent `lock()` // will now return `nullptr`, in which case no work is done. current_timer_.cancel(); + + // Send fire-and-forget `AsyncDeleteSession()` calls for all sessions. + for (auto const& session : sessions_) { + if (session->is_bad()) continue; + AsyncDeleteSession(cq_, session->channel()->stub, session->session_name()) + .then([](auto result) { auto status = result.get(); }); + } } void SessionPool::ScheduleBackgroundWork(std::chrono::seconds relative_time) { diff --git a/google/cloud/spanner/internal/session_pool_test.cc b/google/cloud/spanner/internal/session_pool_test.cc index 87385d5eac0e3..22da5b67f7810 100644 --- a/google/cloud/spanner/internal/session_pool_test.cc +++ b/google/cloud/spanner/internal/session_pool_test.cc @@ -97,6 +97,12 @@ MATCHER_P(SessionCountIs, session_count, return arg.session_count() == session_count; } +// Matches a DeleteSessionRequest with the specified `name`. +MATCHER_P(SessionNameIs, name, + "DeleteSessionRequest has expected session name") { + return arg.name() == name; +} + google::protobuf::Timestamp Now() { auto now = spanner::MakeTimestamp(absl::Now()).value(); return now.get().value(); @@ -142,6 +148,8 @@ TEST_F(SessionPoolTest, AllocateRouteToLeader) { Contains(Pair(kRouteToLeader, "true"))); return MakeSessionsResponse({"session1"}); }); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session1"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = @@ -168,6 +176,8 @@ TEST_F(SessionPoolTest, AllocateNoRouteToLeader) { Not(Contains(Pair(kRouteToLeader, _))))); return MakeSessionsResponse({"session1"}); }); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session1"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = @@ -192,6 +202,10 @@ TEST_F(SessionPoolTest, ReleaseBadSession) { BatchCreateSessions( _, _, AllOf(DatabaseIs(db.FullName()), SessionCountIs(2)))) .WillOnce(Return(ByMove(MakeSessionsResponse({"session2"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session1"))) + .Times(0); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session2"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool( @@ -221,6 +235,7 @@ TEST_F(SessionPoolTest, CreateError) { EXPECT_CALL(*mock, BatchCreateSessions) .WillOnce(Return(ByMove(Status(StatusCode::kInternal, "init failure")))) .WillOnce(Return(ByMove(Status(StatusCode::kInternal, "some failure")))); + EXPECT_CALL(*mock, AsyncDeleteSession).Times(0); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool(db, {mock}, threads.cq()); @@ -234,6 +249,8 @@ TEST_F(SessionPoolTest, ReuseSession) { auto db = spanner::Database("project", "instance", "database"); EXPECT_CALL(*mock, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"session1"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session1"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool(db, {mock}, threads.cq()); @@ -253,6 +270,10 @@ TEST_F(SessionPoolTest, Lifo) { EXPECT_CALL(*mock, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"session1"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"session2"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session2"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool(db, {mock}, threads.cq()); @@ -284,6 +305,12 @@ TEST_F(SessionPoolTest, MinSessionsEagerAllocation) { auto db = spanner::Database("project", "instance", "database"); EXPECT_CALL(*mock, BatchCreateSessions(_, _, SessionCountIs(min_sessions))) .WillOnce(Return(ByMove(MakeSessionsResponse({"s3", "s2", "s1"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s3"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool( @@ -303,6 +330,20 @@ TEST_F(SessionPoolTest, MinSessionsMultipleAllocations) { EXPECT_CALL(*mock, BatchCreateSessions(_, _, SessionCountIs(min_sessions + 1))) .WillOnce(Return(ByMove(MakeSessionsResponse({"s7", "s6", "s5", "s4"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s3"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s4"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s5"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s6"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s7"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool( @@ -328,6 +369,12 @@ TEST_F(SessionPoolTest, MaxSessionsFailOnExhaustion) { .WillOnce(Return(ByMove(MakeSessionsResponse({"s1"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"s2"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"s3"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s3"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool( @@ -357,6 +404,8 @@ TEST_F(SessionPoolTest, MaxSessionsBlockUntilRelease) { auto db = spanner::Database("project", "instance", "database"); EXPECT_CALL(*mock, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"s1"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s1"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool( @@ -388,6 +437,8 @@ TEST_F(SessionPoolTest, Labels) { {"k1", "v1"}, {"k2", "v2"}, {"k3", "v3"}}; EXPECT_CALL(*mock, BatchCreateSessions(_, _, LabelsAre(labels))) .WillOnce(Return(ByMove(MakeSessionsResponse({"session1"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session1"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool( @@ -406,6 +457,8 @@ TEST_F(SessionPoolTest, CreatorRole) { BatchCreateSessions( _, _, AllOf(DatabaseIs(db.FullName()), CreatorRoleIs(role)))) .WillOnce(Return(ByMove(MakeSessionsResponse({"session1"}, role)))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("session1"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool( @@ -424,10 +477,22 @@ TEST_F(SessionPoolTest, MultipleChannels) { .WillOnce(Return(ByMove(MakeSessionsResponse({"c1s1"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"c1s2"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"c1s3"})))); + EXPECT_CALL(*mock1, AsyncDeleteSession(_, _, _, SessionNameIs("c1s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock1, AsyncDeleteSession(_, _, _, SessionNameIs("c1s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock1, AsyncDeleteSession(_, _, _, SessionNameIs("c1s3"))) + .WillOnce(Return(make_ready_future(Status{}))); EXPECT_CALL(*mock2, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"c2s1"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"c2s2"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"c2s3"})))); + EXPECT_CALL(*mock2, AsyncDeleteSession(_, _, _, SessionNameIs("c2s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock2, AsyncDeleteSession(_, _, _, SessionNameIs("c2s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock2, AsyncDeleteSession(_, _, _, SessionNameIs("c2s3"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; auto pool = MakeTestSessionPool(db, {mock1, mock2}, threads.cq()); @@ -450,10 +515,28 @@ TEST_F(SessionPoolTest, MultipleChannelsPreAllocation) { auto db = spanner::Database("project", "instance", "database"); EXPECT_CALL(*mock1, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"c1s1", "c1s2", "c1s3"})))); + EXPECT_CALL(*mock1, AsyncDeleteSession(_, _, _, SessionNameIs("c1s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock1, AsyncDeleteSession(_, _, _, SessionNameIs("c1s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock1, AsyncDeleteSession(_, _, _, SessionNameIs("c1s3"))) + .WillOnce(Return(make_ready_future(Status{}))); EXPECT_CALL(*mock2, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"c2s1", "c2s2", "c2s3"})))); + EXPECT_CALL(*mock2, AsyncDeleteSession(_, _, _, SessionNameIs("c2s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock2, AsyncDeleteSession(_, _, _, SessionNameIs("c2s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock2, AsyncDeleteSession(_, _, _, SessionNameIs("c2s3"))) + .WillOnce(Return(make_ready_future(Status{}))); EXPECT_CALL(*mock3, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"c3s1", "c3s2", "c3s3"})))); + EXPECT_CALL(*mock3, AsyncDeleteSession(_, _, _, SessionNameIs("c3s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock3, AsyncDeleteSession(_, _, _, SessionNameIs("c3s2"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock3, AsyncDeleteSession(_, _, _, SessionNameIs("c3s3"))) + .WillOnce(Return(make_ready_future(Status{}))); google::cloud::internal::AutomaticallyCreatedBackgroundThreads threads; // note that min_sessions will effectively be reduced to 9 @@ -497,6 +580,10 @@ TEST_F(SessionPoolTest, SessionRefresh) { EXPECT_CALL(*mock, BatchCreateSessions) .WillOnce(Return(ByMove(MakeSessionsResponse({"s1"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"s2"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s2"))) + .WillOnce(Return(make_ready_future(Status{}))); google::spanner::v1::ResultSet result; auto constexpr kResultSetText = R"pb( @@ -567,6 +654,11 @@ TEST_F(SessionPoolTest, SessionRefreshNotFound) { .WillOnce(Return(ByMove(MakeSessionsResponse({"s1"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"s2"})))) .WillOnce(Return(ByMove(MakeSessionsResponse({"s3"})))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s1"))) + .WillOnce(Return(make_ready_future(Status{}))); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s2"))).Times(0); + EXPECT_CALL(*mock, AsyncDeleteSession(_, _, _, SessionNameIs("s3"))) + .WillOnce(Return(make_ready_future(Status{}))); EXPECT_CALL(*mock, AsyncExecuteSql) .WillOnce([](CompletionQueue&, auto, auto,