From: Pete Zaitcev <zaitcev@redhat.com>
To: Jeff Garzik <jeff@garzik.org>
Cc: Project Hail List <hail-devel@vger.kernel.org>
Subject: [Patch] tabled: replace event_loopbreak with pipe
Date: Wed, 3 Feb 2010 20:02:07 -0700 [thread overview]
Message-ID: <20100203200207.6235b8d9@redhat.com> (raw)
As it turned out, event_loopbreak() does not awaken the thread that
exectutes event_dispatch(), but our code expected that it would.
One easily noticeable effect was that there was a noticeable delay
between the state transition to DB master and listening on sockets.
I knew the mysterious delay existed for a while, but never got around
to investigate. For ncld API, I moved the processing of CLD packets
to its own thread and suddenly everything else froze. Apparently the
existing code only works because of extra packets of CLD protocol.
For a fix, dispose of event_loopbreak and use a loopback pipe.
Also gone is state_tdb_new. That thing was just disgusting.
Notice that we still have one event_loopbreak remaining, because it
works correctly thanks to UNIX signal awakening the polling thread.
Signed-Off-By: Pete Zaitcev <zaitcev@redhat.com>
---
server/server.c | 90 +++++++++++++++++++++++++++++++++-------------
server/tabled.h | 4 +-
2 files changed, 68 insertions(+), 26 deletions(-)
commit fb1aaf280f14e020e6a0110b828f4879b5eaa11e
Author: Master <zaitcev@lembas.zaitcev.lan>
Date: Wed Feb 3 19:48:42 2010 -0700
Replace event_loopbreak with ev_pipe.
diff --git a/server/server.c b/server/server.c
index e5580c5..b205340 100644
--- a/server/server.c
+++ b/server/server.c
@@ -89,7 +89,6 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state);
static const struct argp argp = { options, parse_opt, NULL, doc };
static bool server_running = true;
-static bool dump_stats;
static bool use_syslog = true;
int debugging = 0;
@@ -99,6 +98,12 @@ struct server tabled_srv = {
struct tabledb tdb;
+enum {
+ TT_CMD_DUMP,
+ TT_CMD_TDBST_MASTER,
+ TT_CMD_TDBST_SLAVE
+};
+
struct compiled_pat patterns[] = {
[pat_auth] =
{ "^AWS (\\w+):(\\S+)", 0, },
@@ -361,8 +366,8 @@ static void term_signal(int signo)
static void stats_signal(int signo)
{
- dump_stats = true;
- event_loopbreak();
+ static const unsigned char cmd = TT_CMD_DUMP;
+ write(tabled_srv.ev_pipe[1], &cmd, 1);
}
#define X(stat) \
@@ -1353,6 +1358,7 @@ static void tdb_checkpoint(int fd, short events, void *userdata)
static void tdb_state_cb(enum db_event event)
{
+ unsigned char cmd;
switch (event) {
case TDB_EV_ELECTED:
@@ -1369,25 +1375,20 @@ static void tdb_state_cb(enum db_event event)
* This callback runs on the context of the replication
* manager thread, and calling any of our functions thus
* turns our program into a multi-threaded one. Instead
- * we do a loopbreak and postpone the processing.
+ * we signal them main thread to do the processing.
*/
if (tabled_srv.state_tdb != ST_TDB_INIT &&
tabled_srv.state_tdb != ST_TDB_OPEN) {
if (event == TDB_EV_MASTER)
- tabled_srv.state_tdb_new = ST_TDB_MASTER;
+ cmd = TT_CMD_TDBST_MASTER;
else
- tabled_srv.state_tdb_new = ST_TDB_SLAVE;
- if (debugging) {
- applog(LOG_DEBUG, "TDB state > %s",
- state_name_tdb[tabled_srv.state_tdb_new]);
- }
- event_loopbreak();
+ cmd = TT_CMD_TDBST_SLAVE;
+ write(tabled_srv.ev_pipe[1], &cmd, 1);
}
break;
default:
applog(LOG_WARNING, "API confusion with TDB, event 0x%x", event);
tabled_srv.state_tdb = ST_TDB_OPEN; /* wrong, stub for now */
- tabled_srv.state_tdb_new = ST_TDB_INIT;
}
}
@@ -1727,6 +1728,8 @@ static void tdb_state_process(enum st_tdb new_state)
{
unsigned int db_flags;
+ if (debugging)
+ applog(LOG_DEBUG, "TDB state > %s", state_name_tdb[new_state]);
if ((new_state == ST_TDB_MASTER || new_state == ST_TDB_SLAVE) &&
tabled_srv.state_tdb == ST_TDB_ACTIVE) {
@@ -1744,6 +1747,38 @@ static void tdb_state_process(enum st_tdb new_state)
}
}
+static void internal_event(int fd, short events, void *userdata)
+{
+ unsigned char cmd;
+ ssize_t rrc;
+
+ rrc = read(tabled_srv.ev_pipe[0], &cmd, 1);
+ if (rrc < 0) {
+ applog(LOG_WARNING, "pipe read error: %s", strerror(errno));
+ abort();
+ }
+ if (rrc < 1) {
+ applog(LOG_WARNING, "pipe short read");
+ abort();
+ }
+
+ if (cmd == TT_CMD_DUMP) {
+ stats_dump();
+ } else if (cmd == TT_CMD_TDBST_MASTER) {
+ if (tabled_srv.state_tdb != ST_TDB_MASTER) {
+ tdb_state_process(ST_TDB_MASTER);
+ tabled_srv.state_tdb = ST_TDB_MASTER;
+ }
+ } else if (cmd == TT_CMD_TDBST_SLAVE) {
+ if (tabled_srv.state_tdb != ST_TDB_SLAVE) {
+ tdb_state_process(ST_TDB_SLAVE);
+ tabled_srv.state_tdb = ST_TDB_SLAVE;
+ }
+ } else {
+ applog(LOG_WARNING, "internal error: command 0x%x", cmd);
+ }
+}
+
int main (int argc, char *argv[])
{
error_t aprc;
@@ -1820,9 +1855,22 @@ int main (int argc, char *argv[])
signal(SIGTERM, term_signal);
signal(SIGUSR1, stats_signal);
+ /*
+ * Prepare the libevent paraphernalia
+ */
tabled_srv.evbase_main = event_init();
event_base_rep = event_base_new();
evtimer_set(&tabled_srv.chkpt_timer, tdb_checkpoint, NULL);
+ if (pipe(tabled_srv.ev_pipe) < 0) {
+ applogerr("pipe");
+ goto err_evpipe;
+ }
+ event_set(&tabled_srv.pevt, tabled_srv.ev_pipe[0], EV_READ | EV_PERSIST,
+ internal_event, NULL);
+ if (event_add(&tabled_srv.pevt, NULL) < 0) {
+ applog(LOG_WARNING, "pevt event_add");
+ goto err_pevt;
+ }
/* set up server networking */
rc = net_open();
@@ -1839,21 +1887,9 @@ int main (int argc, char *argv[])
applog(LOG_INFO, "initialized (%s)",
(tabled_srv.flags & SFL_FOREGROUND)? "fg": "bg");
- while (server_running) {
+ while (server_running)
event_dispatch();
- if (dump_stats) {
- dump_stats = false;
- stats_dump();
- }
-
- if (tabled_srv.state_tdb_new != ST_TDB_INIT &&
- tabled_srv.state_tdb_new != tabled_srv.state_tdb) {
- tdb_state_process(tabled_srv.state_tdb_new);
- tabled_srv.state_tdb = tabled_srv.state_tdb_new;
- }
- }
-
applog(LOG_INFO, "shutting down");
rc = 0;
@@ -1871,6 +1907,10 @@ err_out_net:
tdb_fini(&tdb);
}
/* err_tdb_init: */
+err_pevt:
+ close(tabled_srv.ev_pipe[0]);
+ close(tabled_srv.ev_pipe[1]);
+err_evpipe:
unlink(tabled_srv.pid_file);
close(tabled_srv.pid_fd);
err_out:
diff --git a/server/tabled.h b/server/tabled.h
index a2ffd8b..91cccae 100644
--- a/server/tabled.h
+++ b/server/tabled.h
@@ -226,6 +226,8 @@ struct server {
int pid_fd; /* fd of pid_file */
GMutex *bigmutex;
struct event_base *evbase_main;
+ int ev_pipe[2];
+ struct event pevt;
char *config; /* config file (static) */
@@ -248,7 +250,7 @@ struct server {
int num_stor; /* number of storage_node's */
uint64_t object_count;
- enum st_tdb state_tdb, state_tdb_new;
+ enum st_tdb state_tdb;
enum st_net state_net;
struct event chkpt_timer; /* db4 checkpoint timer */
next reply other threads:[~2010-02-04 3:02 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-04 3:02 Pete Zaitcev [this message]
2010-02-04 5:19 ` [Patch] tabled: replace event_loopbreak with pipe Jeff Garzik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100203200207.6235b8d9@redhat.com \
--to=zaitcev@redhat.com \
--cc=hail-devel@vger.kernel.org \
--cc=jeff@garzik.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.