Skip to content

Commit

Permalink
[#24582] YSQL, ASH: Handle more interrupts in ASH collector
Browse files Browse the repository at this point in the history
Summary:
PG 15 added a new interrupt ProcSignalBarrierPending which is
set to true after certain queries, ProcessProcSignalBarrier()
must be called, otherwise ASH collector is stuck, as seen by
the PgWaitEventAuxTest.NewDatabaseRPCs test.

This test was created before PG 15 merge, and skipped
when D38309 / 72fc467
was landed with PG 15 changes.

Summary of changes -
- Remove signal handlers defined in yb_ash.h
- Add signal handlers for SIGINT and SIGQUIT too
- Use HandleMainLoopInterrupts to handle signals

Jira: DB-13621

Test Plan:
Jenkins

./yb_build.sh --cxx-test pg_ash-test --gtest_filter *NewDatabaseRPCs

Reviewers: jason

Reviewed By: jason

Subscribers: amitanand, hbhanawat, yql

Differential Revision: https://phorge.dev.yugabyte.com/D39533
  • Loading branch information
abhinab-yb committed Nov 4, 2024
1 parent 42e0d9b commit 6e87f27
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 40 deletions.
47 changes: 8 additions & 39 deletions src/postgres/src/backend/utils/misc/yb_ash.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "parser/scansup.h"
#include "pgstat.h"
#include "postmaster/bgworker.h"
#include "postmaster/interrupt.h"
#include "storage/ipc.h"
#include "storage/latch.h"
#include "storage/lwlock.h"
Expand Down Expand Up @@ -81,10 +82,6 @@ static ExecutorFinish_hook_type prev_ExecutorFinish = NULL;
static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
static ProcessUtility_hook_type prev_ProcessUtility = NULL;

/* Flags set by interrupt handlers for later service in the main loop. */
static volatile sig_atomic_t got_sigterm = false;
static volatile sig_atomic_t got_sighup = false;

YbAshTrackNestedQueries yb_ash_track_nested_queries = NULL;

typedef struct YbAsh
Expand Down Expand Up @@ -330,7 +327,7 @@ yb_ash_ExecutorStart(QueryDesc *queryDesc, int eflags)
query_id = queryDesc->plannedstmt->queryId != 0
? queryDesc->plannedstmt->queryId
: yb_ash_utility_query_id(queryDesc->sourceText,
queryDesc->plannedstmt->stmt_len,
queryDesc->plannedstmt->stmt_len,
queryDesc->plannedstmt->stmt_location);
YbAshSetQueryId(query_id);
}
Expand Down Expand Up @@ -676,28 +673,6 @@ YbAshReleaseBufferLock()
LWLockRelease(&yb_ash->lock);
}

static void
yb_ash_sigterm(SIGNAL_ARGS)
{
int save_errno = errno;

got_sigterm = true;
SetLatch(MyLatch);

errno = save_errno;
}

static void
yb_ash_sighup(SIGNAL_ARGS)
{
int save_errno = errno;

got_sighup = true;
SetLatch(MyLatch);

errno = save_errno;
}

void
YbAshMain(Datum main_arg)
{
Expand All @@ -707,8 +682,10 @@ YbAshMain(Datum main_arg)
yb_ash_circular_buffer_size * 1024)));

/* Register functions for SIGTERM/SIGHUP management */
pqsignal(SIGHUP, yb_ash_sighup);
pqsignal(SIGTERM, yb_ash_sigterm);
pqsignal(SIGHUP, SignalHandlerForConfigReload);
pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
pqsignal(SIGINT, SignalHandlerForShutdownRequest);
pqsignal(SIGQUIT, SignalHandlerForCrashExit);

/* We're now ready to receive signals */
BackgroundWorkerUnblockSignals();
Expand All @@ -717,7 +694,7 @@ YbAshMain(Datum main_arg)

pgstat_report_appname("yb_ash collector");

while (!got_sigterm)
while (true)
{
TimestampTz sample_time;
int rc;
Expand All @@ -730,15 +707,7 @@ YbAshMain(Datum main_arg)
if (rc & WL_POSTMASTER_DEATH)
proc_exit(1);

/* Process signals */
if (got_sighup)
{
/* Process config file */
got_sighup = false;
ProcessConfigFile(PGC_SIGHUP);
ereport(LOG,
(errmsg("bgworker yb_ash signal: processed SIGHUP")));
}
HandleMainLoopInterrupts();

if (yb_enable_ash && yb_ash_sample_size > 0)
{
Expand Down
1 change: 0 additions & 1 deletion src/yb/yql/pgwrapper/pg_ash-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ TEST_F(PgAshTest, YB_DISABLE_TEST_IN_TSAN(NoMemoryLeaks)) {
}

TEST_F_EX(PgWaitEventAuxTest, NewDatabaseRPCs, PgNewDatabaseWaitEventAux) {
GTEST_SKIP() << "Skipping this test until #24582 is done";
ASSERT_OK(conn_->Execute("CREATE DATABASE db1"));
ASSERT_OK(conn_->Execute("ALTER DATABASE db1 RENAME TO db2"));
ASSERT_OK(conn_->Execute("DROP DATABASE db2"));
Expand Down

0 comments on commit 6e87f27

Please sign in to comment.