Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

params: move blocking read to Cython and remove C++ signal handling #34407

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 2 additions & 27 deletions common/params.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include <algorithm>
#include <cassert>
#include <csignal>
#include <unordered_map>

#include "common/queue.h"
Expand All @@ -15,11 +14,6 @@

namespace {

volatile sig_atomic_t params_do_exit = 0;
void params_sig_handler(int signal) {
params_do_exit = 1;
}

int fsync_dir(const std::string &path) {
int result = -1;
int fd = HANDLE_EINTR(open(path.c_str(), O_RDONLY, 0755));
Expand Down Expand Up @@ -282,27 +276,8 @@ int Params::remove(const std::string &key) {
return fsync_dir(getParamPath());
}

std::string Params::get(const std::string &key, bool block) {
if (!block) {
return util::read_file(getParamPath(key));
} else {
// blocking read until successful
params_do_exit = 0;
void (*prev_handler_sigint)(int) = std::signal(SIGINT, params_sig_handler);
void (*prev_handler_sigterm)(int) = std::signal(SIGTERM, params_sig_handler);

std::string value;
while (!params_do_exit) {
if (value = util::read_file(getParamPath(key)); !value.empty()) {
break;
}
util::sleep_for(100); // 0.1 s
}

std::signal(SIGINT, prev_handler_sigint);
std::signal(SIGTERM, prev_handler_sigterm);
return value;
}
std::string Params::get(const std::string &key) {
return util::read_file(getParamPath(key));
}

std::map<std::string, std::string> Params::readAll() {
Expand Down
6 changes: 3 additions & 3 deletions common/params.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ class Params {
void clearAll(ParamKeyType type);

// helpers for reading values
std::string get(const std::string &key, bool block = false);
inline bool getBool(const std::string &key, bool block = false) {
return get(key, block) == "1";
std::string get(const std::string &key);
inline bool getBool(const std::string &key) {
return get(key) == "1";
}
std::map<std::string, std::string> readAll();

Expand Down
27 changes: 13 additions & 14 deletions common/params_pyx.pyx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# distutils: language = c++
# cython: language_level = 3
import time
from libcpp cimport bool
from libcpp.string cimport string
from libcpp.vector cimport vector
Expand All @@ -15,8 +16,8 @@ cdef extern from "common/params.h":

cdef cppclass c_Params "Params":
c_Params(string) except + nogil
string get(string, bool) nogil
bool getBool(string, bool) nogil
string get(string) nogil
bool getBool(string) nogil
int remove(string) nogil
int put(string, string) nogil
void putNonBlocking(string, string) nogil
Expand Down Expand Up @@ -63,24 +64,22 @@ cdef class Params:
cdef string k = self.check_key(key)
cdef string val
with nogil:
val = self.p.get(k, block)
val = self.p.get(k)

if val == b"":
if block:
# If we got no value while running in blocked mode
# it means we got an interrupt while waiting
raise KeyboardInterrupt
else:
return None
# If blocking is enabled, loop until a non-empty value is retrieved
while block and val == b"":
time.sleep(0.1)
with nogil:
val = self.p.get(k)

if val == b"":
return None
return val if encoding is None else val.decode(encoding)

def get_bool(self, key, bool block=False):
cdef string k = self.check_key(key)
cdef bool r
with nogil:
r = self.p.getBool(k, block)
return r
val = self.get(k, block)
return True if val == b"1" else False

def put(self, key, dat):
"""
Expand Down