Skip to content

Commit 5563c09

Browse files
authored
Merge pull request ClickHouse#76228 from ClickHouse/backport/24.8/75062
Backport ClickHouse#75062 to 24.8: Fix crash due to uncaught exception in PostgreSQLReplicationHandler::cleanupFunc()
2 parents b95ad8e + 12e610b commit 5563c09

File tree

1 file changed

+23
-14
lines changed

1 file changed

+23
-14
lines changed

src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ void PostgreSQLReplicationHandler::checkConnectionAndStart()
223223
}
224224
catch (const pqxx::broken_connection & pqxx_error)
225225
{
226-
tryLogCurrentException(__PRETTY_FUNCTION__);
226+
tryLogCurrentException(log);
227227

228228
if (!is_attach)
229229
throw;
@@ -233,7 +233,7 @@ void PostgreSQLReplicationHandler::checkConnectionAndStart()
233233
}
234234
catch (...)
235235
{
236-
tryLogCurrentException(__PRETTY_FUNCTION__);
236+
tryLogCurrentException(log);
237237

238238
if (!is_attach)
239239
throw;
@@ -310,7 +310,7 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error)
310310
catch (Exception & e)
311311
{
312312
e.addMessage("while loading table `{}`.`{}`", postgres_database, table_name);
313-
tryLogCurrentException(__PRETTY_FUNCTION__);
313+
tryLogCurrentException(log);
314314

315315
/// Throw in case of single MaterializedPostgreSQL storage, because initial setup is done immediately
316316
/// (unlike database engine where it is done in a separate thread).
@@ -358,7 +358,7 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error)
358358
catch (Exception & e)
359359
{
360360
e.addMessage("while loading table {}.{}", postgres_database, table_name);
361-
tryLogCurrentException(__PRETTY_FUNCTION__);
361+
tryLogCurrentException(log);
362362

363363
if (throw_on_error)
364364
throw;
@@ -468,16 +468,25 @@ StorageInfo PostgreSQLReplicationHandler::loadFromSnapshot(postgres::Connection
468468

469469
void PostgreSQLReplicationHandler::cleanupFunc()
470470
{
471-
/// It is very important to make sure temporary replication slots are removed!
472-
/// So just in case every 30 minutes check if one still exists.
473-
postgres::Connection connection(connection_info);
474-
String last_committed_lsn;
475-
connection.execWithRetry([&](pqxx::nontransaction & tx)
471+
try
476472
{
477-
if (isReplicationSlotExist(tx, last_committed_lsn, /* temporary */true))
478-
dropReplicationSlot(tx, /* temporary */true);
479-
});
480-
cleanup_task->scheduleAfter(CLEANUP_RESCHEDULE_MS);
473+
/// It is very important to make sure temporary replication slots are removed!
474+
/// So just in case every 30 minutes check if one still exists.
475+
postgres::Connection connection(connection_info);
476+
String last_committed_lsn;
477+
connection.execWithRetry([&](pqxx::nontransaction & tx)
478+
{
479+
if (isReplicationSlotExist(tx, last_committed_lsn, /* temporary */true))
480+
dropReplicationSlot(tx, /* temporary */true);
481+
});
482+
}
483+
catch (...)
484+
{
485+
tryLogCurrentException(log);
486+
}
487+
488+
if (!stop_synchronization)
489+
cleanup_task->scheduleAfter(CLEANUP_RESCHEDULE_MS);
481490
}
482491

483492
PostgreSQLReplicationHandler::ConsumerPtr PostgreSQLReplicationHandler::getConsumer()
@@ -498,7 +507,7 @@ void PostgreSQLReplicationHandler::consumerFunc()
498507
}
499508
catch (...)
500509
{
501-
tryLogCurrentException(__PRETTY_FUNCTION__);
510+
tryLogCurrentException(log);
502511
}
503512

504513
if (stop_synchronization)

0 commit comments

Comments
 (0)