Skip to content

Commit

Permalink
[yugabyte#20028] YSQL: Set webserver OOM score to yb_webserver_oom_sc…
Browse files Browse the repository at this point in the history
…ore_adj (default 900)

Summary:
Prior to this diff, the webserver's oom_score_adj is 0, while PG backends are 900. If the webserver started consuming too much memory, all PG backends were killed before we kill the webserver.

Now, set to the value passed in by the flag `yb_webserver_oom_score_adj` (similar to `yb_backend_oom_score_adj`). Both default to 900.

We use the same score because typically the webserver just uses 30-40mb. If the webserver oom_score_adj was higher than the backends, it would be the first process to be killed whenever we are in a tight spot. The postmaster would immediately recreate it, and the webserver would go back to consuming 30-40mb of memory. So in a "normal" OOM case where a backend is consuming too much memory, we would unnecessarily and uselessly kill the webserver multiple times. In the case where the webserver is using a large mount of memory, the oom killer will target it first anyways. At the very least, it will be among the first processes to be killed.

Setting webserver's oom_score_adj = backend's oom_score_adj means we don't unnecessarily kill the webserver, and we'll quickly kill the webserver if it becomes a problem anyways.
Jira: DB-8994

Test Plan:
```
pid=$(pgrep -f "YSQL webserver"); cat /proc/$pid/oom_score_adj # shows 900
pkill -f -9 "YSQL webserver" # kill the webserver to test the oom score when it's restarted
pid=$(pgrep -f "YSQL webserver"); cat /proc/$pid/oom_score_adj # shows 900
```

And, automated tests:
```
./yb_build.sh --cxx-test pgwrapper_pg_libpq-test --gtest_filter PgLibPqTest.TestOomScoreAdjPGBackend
./yb_build.sh --cxx-test pgwrapper_pg_libpq-test --gtest_filter PgLibPqTest.TestOomScoreAdjPGWebserver # new test!
```

Reviewers: smishra, kfranz, amartsinchyk

Reviewed By: smishra

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D30510
  • Loading branch information
timothy-e committed Nov 29, 2023
1 parent da6e4cf commit 5e732e2
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 34 deletions.
3 changes: 3 additions & 0 deletions src/postgres/contrib/yb_pg_metrics/yb_pg_metrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,9 @@ _PG_init(void)
strcpy(worker.bgw_library_name, "yb_pg_metrics");
strcpy(worker.bgw_function_name, "webserver_worker_main");
worker.bgw_notify_pid = 0;
if (getenv("FLAGS_yb_webserver_oom_score_adj") != NULL)
worker.bgw_oom_score_adj = strdup(getenv("FLAGS_yb_webserver_oom_score_adj"));

RegisterBackgroundWorker(&worker);
/*
* Set the value of the hooks.
Expand Down
67 changes: 38 additions & 29 deletions src/postgres/src/backend/postmaster/postmaster.c
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,7 @@ static bool FatalError = false; /* T if recovering from backend crash */
/* Crashed before fully acquiring a lock, or with unexpected error code. */
static bool YbCrashInUnmanageableState = false;

#ifdef __linux__
static char *YbBackendOomScoreAdj = NULL;
#endif

/*
* We use a simple state machine to control startup, shutdown, and
Expand Down Expand Up @@ -4264,6 +4262,41 @@ TerminateChildren(int signal)
signal_child(PgStatPID, signal);
}

/*
* SetOomScoreAdjForPid - sets /proc/<pid>/oom_score_adj for the given PID
*
* oom_score_adj varies from -1000 to 1000. The lower the value, the lower the
* chance that it's going to be killed. A high value is more likely to be
* killed by the OOM killer.
*/
static void
SetOomScoreAdjForPid(pid_t pid, char *oom_score_adj)
{
#ifdef __linux__
if (oom_score_adj == NULL)
return;

char file_name[64];
snprintf(file_name, sizeof(file_name), "/proc/%d/oom_score_adj", pid);
FILE * fPtr;
fPtr = fopen(file_name, "w");

if(fPtr == NULL)
{
int saved_errno = errno;
ereport(LOG,
(errcode_for_file_access(),
errmsg("error %d: %s, unable to open file %s", saved_errno,
strerror(saved_errno), file_name)));
}
else
{
fputs(oom_score_adj, fPtr);
fclose(fPtr);
}
#endif
}

/*
* BackendStartup -- start backend process
*
Expand Down Expand Up @@ -4387,33 +4420,7 @@ BackendStartup(Port *port)
ShmemBackendArrayAdd(bn);
#endif

#ifdef __linux__
char file_name[64];
snprintf(file_name, sizeof(file_name), "/proc/%d/oom_score_adj", pid);
FILE * fPtr;
fPtr = fopen(file_name, "w");

/*
* oom_score_adj varies from -1000 to 1000. The lower the value, the lower
* the chance that it's going to be killed. Here, we are setting low priority
* (YbBackendOomScoreAdj) for postgres connections so that during out of
* memory, postgres connections are killed first. We do that be setting a
* high oom_score_adj value for the postgres connection.
*/
if(fPtr == NULL)
{
int saved_errno = errno;
ereport(LOG,
(errcode_for_file_access(),
errmsg("error %d: %s, unable to open file %s", saved_errno,
strerror(saved_errno), file_name)));
}
else
{
fputs(YbBackendOomScoreAdj, fPtr);
fclose(fPtr);
}
#endif
SetOomScoreAdjForPid(pid, YbBackendOomScoreAdj);

return STATUS_OK;
}
Expand Down Expand Up @@ -6069,6 +6076,8 @@ do_start_bgworker(RegisteredBgWorker *rw)
MemoryContextDelete(PostmasterContext);
PostmasterContext = NULL;

SetOomScoreAdjForPid(MyProcPid, rw->rw_worker.bgw_oom_score_adj);

StartBackgroundWorker();

exit(1); /* should not get here */
Expand Down
1 change: 1 addition & 0 deletions src/postgres/src/include/postmaster/bgworker.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ typedef struct BackgroundWorker
Datum bgw_main_arg;
char bgw_extra[BGW_EXTRALEN];
pid_t bgw_notify_pid; /* SIGUSR1 this backend on start/stop */
char *bgw_oom_score_adj; /* ignored if null */
} BackgroundWorker;

typedef enum BgwHandleStatus
Expand Down
33 changes: 30 additions & 3 deletions src/yb/yql/pgwrapper/pg_libpq-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#include "yb/util/cast.h"
#include "yb/util/metrics.h"
#include "yb/util/monotime.h"
#include "yb/util/os-util.h"
#include "yb/util/path_util.h"
#include "yb/util/random_util.h"
#include "yb/util/scope_exit.h"
Expand Down Expand Up @@ -3106,11 +3107,14 @@ class PgLibPqYSQLBackendCrash: public PgLibPqTest {
options->extra_tserver_flags.push_back(
Format("--TEST_yb_lwlock_crash_after_acquire_pg_stat_statements_reset=true"));
options->extra_tserver_flags.push_back(
Format("--yb_backend_oom_score_adj=" + expected_oom_score));
Format("--yb_backend_oom_score_adj=" + expected_backend_oom_score));
options->extra_tserver_flags.push_back(
Format("--yb_webserver_oom_score_adj=" + expected_webserver_oom_score));
}

protected:
const std::string expected_oom_score = "123";
const std::string expected_backend_oom_score = "123";
const std::string expected_webserver_oom_score = "456";
};

TEST_F_EX(PgLibPqTest,
Expand Down Expand Up @@ -3140,7 +3144,30 @@ TEST_F_EX(PgLibPqTest,
std::ifstream fPtr(file_name);
std::string oom_score_adj;
getline(fPtr, oom_score_adj);
ASSERT_EQ(oom_score_adj, expected_oom_score);
ASSERT_EQ(oom_score_adj, expected_backend_oom_score);
}
TEST_F_EX(PgLibPqTest,
TestOomScoreAdjPGWebserver,
PgLibPqYSQLBackendCrash) {

// Find the postmaster pid (parent process of our webserver)
string postmaster_pid;
auto conn = ASSERT_RESULT(Connect());
auto backend_pid = ASSERT_RESULT(conn.FetchRow<int32_t>("SELECT pg_backend_pid()"));
RunShellProcess(Format("ps -o ppid= $0", backend_pid), &postmaster_pid);

// Get the webserver pid using postmaster pid
string webserver_pid;
RunShellProcess(Format("pgrep -f 'YSQL webserver' -P $0", postmaster_pid), &webserver_pid);
webserver_pid.erase(std::remove(webserver_pid.begin(), webserver_pid.end(), '\n'),
webserver_pid.cend());

// Check the webserver's OOM score
std::string file_name = "/proc/" + webserver_pid + "/oom_score_adj";
std::ifstream fPtr(file_name);
std::string oom_score_adj;
getline(fPtr, oom_score_adj);
ASSERT_EQ(oom_score_adj, expected_webserver_oom_score);
}
#endif

Expand Down
7 changes: 5 additions & 2 deletions src/yb/yql/pgwrapper/pg_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ DEFINE_UNKNOWN_string(pg_proxy_bind_address, "", "Address for the PostgreSQL pro
DEFINE_UNKNOWN_string(postmaster_cgroup, "", "cgroup to add postmaster process to");
DEFINE_UNKNOWN_bool(pg_transactions_enabled, true,
"True to enable transactions in YugaByte PostgreSQL API.");
DEFINE_UNKNOWN_string(yb_backend_oom_score_adj, "900",
DEFINE_NON_RUNTIME_string(yb_backend_oom_score_adj, "900",
"oom_score_adj of postgres backends in linux environments");
DEFINE_UNKNOWN_bool(yb_pg_terminate_child_backend, false,
DEFINE_NON_RUNTIME_string(yb_webserver_oom_score_adj, "900",
"oom_score_adj of YSQL webserver in linux environments");
DEFINE_NON_RUNTIME_bool(yb_pg_terminate_child_backend, false,
"Terminate other active server processes when a backend is killed");
DEFINE_UNKNOWN_bool(pg_verbose_error_log, false,
"True to enable verbose logging of errors in PostgreSQL server");
Expand Down Expand Up @@ -656,6 +658,7 @@ Status PgWrapper::Start() {
proc_->SetEnv("FLAGS_yb_pg_terminate_child_backend",
FLAGS_yb_pg_terminate_child_backend ? "true" : "false");
proc_->SetEnv("FLAGS_yb_backend_oom_score_adj", FLAGS_yb_backend_oom_score_adj);
proc_->SetEnv("FLAGS_yb_webserver_oom_score_adj", FLAGS_yb_webserver_oom_score_adj);

// Pass down custom temp path through environment variable.
if (!VERIFY_RESULT(Env::Default()->DoesDirectoryExist(FLAGS_tmp_dir))) {
Expand Down

0 comments on commit 5e732e2

Please sign in to comment.