Skip to content

Commit 97cf867

Browse files
maxgeorgfacebook-github-bot
authored andcommitted
Replace unique_ptr to db::ConnectionContextBase object with a shared pointer in ConnectOperation and Connection classes.
Summary: `ConnectionOperation::connection_context_` field accumulates some logging information while the connection operation is in flighjt. This diff preserves `connection_context_` pointers in operation and connection objects so the connection context can be accessed through any of them at the end of the operation. Reviewed By: jkedgar Differential Revision: D33663744 fbshipit-source-id: b35ba2dfb40c602b6a6815b988b8141278558c88
1 parent bae0cbf commit 97cf867

File tree

7 files changed

+17
-27
lines changed

7 files changed

+17
-27
lines changed

squangle/mysql_client/AsyncConnectionPool.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -296,16 +296,12 @@ void AsyncConnectionPool::openNewConnection(
296296
// Sanity check
297297
DCHECK(pool_op != nullptr);
298298
conn_storage_.queueOperation(poolKey, pool_op);
299-
// Copy the ConnectionContext from the incoming operation. These contexts
299+
// Propagate the ConnectionContext from the incoming operation. These contexts
300300
// contain application specific logging that will be lost if not passed to
301301
// the new ConnectOperation that is spawned to fulfill the pool miss.
302-
// Creating a copy ensures that both operations are logged with the
303-
// expected additional logging
304-
std::unique_ptr<db::ConnectionContextBase> context;
305-
if (pool_op->connection_context_) {
306-
context = pool_op->connection_context_->copy();
307-
}
308-
tryRequestNewConnection(poolKey, std::move(context));
302+
// Propagating the context pointer ensures that both operations are logged
303+
// with the expected additional logging
304+
tryRequestNewConnection(poolKey, pool_op->connection_context_);
309305
}
310306

311307
void AsyncConnectionPool::reuseConnWithChangeUser(
@@ -562,7 +558,7 @@ void AsyncConnectionPool::connectionSpotFreed(const PoolKey& pool_key) {
562558

563559
void AsyncConnectionPool::tryRequestNewConnection(
564560
const PoolKey& pool_key,
565-
std::unique_ptr<db::ConnectionContextBase> context) {
561+
std::shared_ptr<db::ConnectionContextBase> context) {
566562
// Only called internally, this doesn't need to check if it's shutting
567563
// down
568564
DCHECK_EQ(std::this_thread::get_id(), mysql_client_->threadId());
@@ -581,7 +577,7 @@ void AsyncConnectionPool::tryRequestNewConnection(
581577
auto connOp = mysql_client_->beginConnection(pool_key.connKey);
582578
connOp->setConnectionOptions(pool_key.connOptions);
583579
if (!context) {
584-
context = std::make_unique<db::ConnectionContextBase>();
580+
context = std::make_shared<db::ConnectionContextBase>();
585581
}
586582
connOp->setConnectionContext(std::move(context));
587583
auto pool_ptr = getSelfWeakPointer();

squangle/mysql_client/AsyncConnectionPool.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ class AsyncConnectionPool
569569
// these connections.
570570
void tryRequestNewConnection(
571571
const PoolKey& pool_key,
572-
std::unique_ptr<db::ConnectionContextBase> context = nullptr);
572+
std::shared_ptr<db::ConnectionContextBase> context = nullptr);
573573

574574
// Used for when we fail to open a requested connection. In case of mysql
575575
// failure (e.g. bad password) we propagate the error to all queued

squangle/mysql_client/Connection.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ class Connection {
531531
}
532532
}
533533

534-
void setConnectionContext(std::unique_ptr<db::ConnectionContextBase>&& e) {
534+
void setConnectionContext(std::shared_ptr<db::ConnectionContextBase> e) {
535535
connection_context_ = std::move(e);
536536
}
537537

@@ -543,7 +543,7 @@ class Connection {
543543
bool killOnQueryTimeout_ = false;
544544

545545
// Context information for logging purposes.
546-
std::unique_ptr<db::ConnectionContextBase> connection_context_;
546+
std::shared_ptr<db::ConnectionContextBase> connection_context_;
547547

548548
// Unowned pointer to the client we're from.
549549
MysqlClientBase* mysql_client_;

squangle/mysql_client/MysqlConnectionHolder.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,13 @@ MysqlConnectionHolder::MysqlConnectionHolder(
3131
const ConnectionKey* connKey)
3232
: client_(from_holder->client_),
3333
conn_key_((connKey) ? *connKey : from_holder->conn_key_),
34-
conn_context_(nullptr),
34+
conn_context_(from_holder->conn_context_),
3535
creation_time_(from_holder->creation_time_),
3636
last_activity_time_(from_holder->last_activity_time_),
3737
connection_opened_(from_holder->connection_opened_),
3838
can_reuse_(from_holder->can_reuse_) {
3939
mysql_ = from_holder->stealMysql();
4040
client_->activeConnectionAdded(&conn_key_);
41-
if (from_holder->conn_context_) {
42-
conn_context_ = from_holder->conn_context_->copy();
43-
}
4441
}
4542

4643
bool MysqlConnectionHolder::inTransaction() {

squangle/mysql_client/MysqlConnectionHolder.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class MysqlConnectionHolder {
9494
}
9595

9696
void setConnectionContext(
97-
std::unique_ptr<db::ConnectionContextBase> conn_context) {
97+
std::shared_ptr<db::ConnectionContextBase> conn_context) {
9898
conn_context_ = std::move(conn_context);
9999
}
100100

@@ -129,7 +129,7 @@ class MysqlConnectionHolder {
129129
// notification of completed operations.
130130
MYSQL* mysql_;
131131
const ConnectionKey conn_key_;
132-
std::unique_ptr<db::ConnectionContextBase> conn_context_;
132+
std::shared_ptr<db::ConnectionContextBase> conn_context_;
133133
Timepoint creation_time_;
134134
Timepoint last_activity_time_;
135135
bool connection_opened_ = false;

squangle/mysql_client/Operation.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -733,10 +733,7 @@ void ConnectOperation::socketActionable() {
733733
}
734734
}
735735
conn()->socketHandler()->changeHandlerFD(socket);
736-
if (connection_context_) {
737-
conn()->mysqlConnection()->setConnectionContext(
738-
connection_context_->copy());
739-
}
736+
conn()->mysqlConnection()->setConnectionContext(connection_context_);
740737
conn()->mysqlConnection()->connectionOpened();
741738
attemptSucceeded(OperationResult::Succeeded);
742739
} else {
@@ -883,7 +880,7 @@ void ConnectOperation::specializedCompleteOperation() {
883880

884881
conn()->setConnectionOptions(conn_options_);
885882
conn()->setKillOnQueryTimeout(getKillOnQueryTimeout());
886-
conn()->setConnectionContext(std::move(connection_context_));
883+
conn()->setConnectionContext(connection_context_);
887884

888885
conn()->notify();
889886

squangle/mysql_client/Operation.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ class Operation : public std::enable_shared_from_this<Operation> {
793793

794794
folly::Optional<folly::dynamic> user_data_;
795795
ObserverCallback observer_callback_;
796-
std::unique_ptr<db::ConnectionContextBase> connection_context_;
796+
std::shared_ptr<db::ConnectionContextBase> connection_context_;
797797

798798
MysqlClientBase* mysql_client_;
799799

@@ -862,7 +862,7 @@ class ConnectOperation : public Operation {
862862
// operation will create.
863863
ConnectOperation* setDefaultQueryTimeout(Duration t);
864864
ConnectOperation* setConnectionContext(
865-
std::unique_ptr<db::ConnectionContextBase>&& e) {
865+
std::shared_ptr<db::ConnectionContextBase> e) {
866866
CHECK_THROW(
867867
state_ == OperationState::Unstarted, db::OperationStateException);
868868
connection_context_ = std::move(e);
@@ -1022,7 +1022,7 @@ class ConnectOperation : public Operation {
10221022
ConnectionOptions conn_options_;
10231023

10241024
// Context information for logging purposes.
1025-
std::unique_ptr<db::ConnectionContextBase> connection_context_;
1025+
std::shared_ptr<db::ConnectionContextBase> connection_context_;
10261026

10271027
private:
10281028
void specializedRunImpl();

0 commit comments

Comments
 (0)