[SCM] SAMBA-CTDB repository - branch v3-4-ctdb updated - 3.4.2-ctdb-18-3-g83ca499
Michael Adam
obnox at samba.org
Mon Jan 25 08:55:32 MST 2010
The branch, v3-4-ctdb has been updated
via 83ca4995b1afb9ee57ef5c3610b35ee05af8fbf1 (commit)
via 2b8ad811f1679659753be763684f379e20f2a142 (commit)
via 67ffcac02fb8d4ffc2a22e061bc01fd1b62296cb (commit)
from 3495e4ab6b911e5c6777c62a51ad94a2b6e9d323 (commit)
http://gitweb.samba.org/?p=obnox/samba-ctdb.git;a=shortlog;h=v3-4-ctdb
- Log -----------------------------------------------------------------
commit 83ca4995b1afb9ee57ef5c3610b35ee05af8fbf1
Author: Michael Adam <obnox at samba.org>
Date: Sat Jan 23 01:17:06 2010 +0100
s3:g_lock: remove a nested event loop, replacing the inner loop by select
This made smbd crash in g_lock_lock() when trying to start a
transaction on a db with an already started transaction,
e.g. in a tcon_and_X where the share_info.tdb was not yet
initialized but share_info.tdb was already locked by another
process or writing acces to the winreg rpc pipe where the
registry tdb was already locked by another process.
What we really _want_ to do here by design is to react to
MSG_DBWRAP_G_LOCK_RETRY messages that are either sent
by a client doing g_lock_unlock or by ourselves when
we receive a CTDB_SRVID_SAMBA_NOTIFY or
CTDB_SRVID_RECONFIGURE message from ctdbd, i.e. when
either a client holding a lock or a complete node
has died.
Doing this properly involves calling tevent_loop_once(),
but doing this here with the main ctdbd messaging context
creates a nested event loop when g_lock_lock() is called
from the main event loop.
So as a quick fix, we act a little corasely here: we do
a select on the ctdb connection fd and when it is readable
or we get EINTR, then we retry without actually parsing
any ctdb packages or dispatching messages. This means that
we retry more often than necessary and intended by design,
but this does not harm and it is unobtrusive. When we have
finished, the main loop will pick up all the messages and
ctdb packets. The only extra twist is that we cannot use
timed events here but have to handcode a timeout for select.
Michael
commit 2b8ad811f1679659753be763684f379e20f2a142
Author: Michael Adam <obnox at samba.org>
Date: Sat Jan 23 00:05:15 2010 +0100
s3:ctdb_conn: add ctdbd_conn_get_fd() to get the fd out of the ctdb connection
Michael
commit 67ffcac02fb8d4ffc2a22e061bc01fd1b62296cb
Author: Michael Adam <obnox at samba.org>
Date: Fri Jan 22 15:56:28 2010 +0100
s3:g_lock: remove an unreached code path.
Michael
-----------------------------------------------------------------------
Summary of changes:
source3/include/ctdbd_conn.h | 2 +
source3/lib/ctdbd_conn.c | 5 ++
source3/lib/g_lock.c | 141 +++++++++++++++++++++++++++++------------
3 files changed, 107 insertions(+), 41 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h
index 71516c7..c5ba572 100644
--- a/source3/include/ctdbd_conn.h
+++ b/source3/include/ctdbd_conn.h
@@ -33,6 +33,8 @@ NTSTATUS ctdbd_register_msg_ctx(struct ctdbd_connection *conn,
struct messaging_context *msg_ctx);
struct messaging_context *ctdb_conn_msg_ctx(struct ctdbd_connection *conn);
+int ctdbd_conn_get_fd(struct ctdbd_connection *conn);
+
NTSTATUS ctdbd_messaging_send(struct ctdbd_connection *conn,
uint32 dst_vnn, uint64 dst_srvid,
struct messaging_rec *msg);
diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index a00c510..3983b3d 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -578,6 +578,11 @@ struct messaging_context *ctdb_conn_msg_ctx(struct ctdbd_connection *conn)
return conn->msg_ctx;
}
+int ctdbd_conn_get_fd(struct ctdbd_connection *conn)
+{
+ return packet_get_fd(conn->pkt);
+}
+
/*
* Packet handler to receive and handle a ctdb message
*/
diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 3905a80..9971d71 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -266,7 +266,9 @@ NTSTATUS g_lock_lock(struct g_lock_ctx *ctx, const char *name,
struct tevent_timer *te = NULL;
NTSTATUS status;
bool retry = false;
- bool timedout = false;
+ struct timeval timeout_end;
+ struct timeval timeout_remaining;
+ struct timeval time_now;
DEBUG(10, ("Trying to acquire lock %d for %s\n", (int)lock_type,
name));
@@ -295,56 +297,113 @@ NTSTATUS g_lock_lock(struct g_lock_ctx *ctx, const char *name,
nt_errstr(status)));
return status;
}
-again:
- retry = false;
- status = g_lock_trylock(ctx, name, lock_type);
- if (NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("Got lock %s\n", name));
- goto done;
- }
- if (!NT_STATUS_EQUAL(status, STATUS_PENDING)) {
- DEBUG(10, ("g_lock_trylock failed: %s\n",
- nt_errstr(status)));
- goto done;
- }
+ time_now = timeval_current();
+ timeout_end = timeval_sum(&time_now, &timeout);
- if (retry) {
- goto again;
- }
+ while (true) {
+ fd_set _r_fds;
+ fd_set *r_fds = NULL;
+ int max_fd = 0;
+ int ret;
+
+ status = g_lock_trylock(ctx, name, lock_type);
+ if (NT_STATUS_IS_OK(status)) {
+ DEBUG(10, ("Got lock %s\n", name));
+ break;
+ }
+ if (!NT_STATUS_EQUAL(status, STATUS_PENDING)) {
+ DEBUG(10, ("g_lock_trylock failed: %s\n",
+ nt_errstr(status)));
+ break;
+ }
- DEBUG(10, ("g_lock_trylock: Did not get lock, waiting...\n"));
+ DEBUG(10, ("g_lock_trylock: Did not get lock, waiting...\n"));
+
+ /* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * !!! HACK ALERT --- FIX ME !!!
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * What we really want to do here is to react to
+ * MSG_DBWRAP_G_LOCK_RETRY messages that are either sent
+ * by a client doing g_lock_unlock or by ourselves when
+ * we receive a CTDB_SRVID_SAMBA_NOTIFY or
+ * CTDB_SRVID_RECONFIGURE message from ctdbd, i.e. when
+ * either a client holding a lock or a complete node
+ * has died.
+ *
+ * Doing this properly involves calling tevent_loop_once(),
+ * but doing this here with the main ctdbd messaging context
+ * creates a nested event loop when g_lock_lock() is called
+ * from the main event loop, e.g. in a tcon_and_X where the
+ * share_info.tdb needs to be initialized and is locked by
+ * another process, or when the remore registry is accessed
+ * for writing and some other process already holds a lock
+ * on the registry.tdb.
+ *
+ * So as a quick fix, we act a little corasely here: we do
+ * a select on the ctdb connection fd and when it is readable
+ * or we get EINTR, then we retry without actually parsing
+ * any ctdb packages or dispatching messages. This means that
+ * we retry more often than intended by design, but this does
+ * not harm and it is unobtrusive. When we have finished,
+ * the main loop will pick up all the messages and ctdb
+ * packets. The only extra twist is that we cannot use timed
+ * events here but have to handcode a timeout.
+ */
- if (te == NULL) {
- te = tevent_add_timer(
- ctx->msg->event_ctx, talloc_tos(),
- timeval_current_ofs(timeout.tv_sec, timeout.tv_usec),
- g_lock_timedout, &timedout);
- if (te == NULL) {
- DEBUG(10, ("tevent_add_timer failed\n"));
- status = NT_STATUS_NO_MEMORY;
- goto done;
- }
- }
+#ifdef CLUSTER_SUPPORT
+ if (lp_clustering()) {
+ struct ctdbd_connection *conn = messaging_ctdbd_connection();
- while (true) {
- if (tevent_loop_once(ctx->msg->event_ctx) == -1) {
- DEBUG(1, ("tevent_loop_once failed\n"));
- status = NT_STATUS_INTERNAL_ERROR;
- goto done;
- }
- if (retry) {
- goto again;
+ r_fds = &_r_fds;
+ FD_ZERO(r_fds);
+ max_fd = ctdbd_conn_get_fd(conn);
+ FD_SET(max_fd, r_fds);
}
- if (timedout) {
- DEBUG(10, ("g_lock_lock timed out\n"));
+#endif
- te = NULL;
+ time_now = timeval_current();
+ timeout_remaining = timeval_until(&time_now, &timeout_end);
- status = NT_STATUS_LOCK_NOT_GRANTED;
- goto done;
+ ret = sys_select(max_fd + 1, r_fds, NULL, NULL,
+ &timeout_remaining);
+
+ if (ret == -1) {
+ if (errno != EINTR) {
+ DEBUG(1, ("error calling select: %s\n",
+ strerror(errno)));
+ status = NT_STATUS_INTERNAL_ERROR;
+ break;
+ }
+ /*
+ * errno == EINTR:
+ * This means a signal was received.
+ * It might have been a MSG_DBWRAP_G_LOCK_RETRY message.
+ * ==> retry
+ */
+ } else if (ret == 0) {
+ if (timeval_expired(&timeout_end)) {
+ DEBUG(10, ("g_lock_lock timed out\n"));
+ status = NT_STATUS_LOCK_NOT_GRANTED;
+ break;
+ } else {
+ DEBUG(10, ("select returned 0 but timeout not "
+ "not expired: strange - retrying\n"));
+ }
+ } else if (ret != 1) {
+ DEBUG(1, ("invalid return code of select: %d\n", ret));
+ status = NT_STATUS_INTERNAL_ERROR;
+ break;
}
+ /*
+ * ret == 1:
+ * This means ctdbd has sent us some data.
+ * Might be a CTDB_SRVID_RECONFIGURE or a
+ * CTDB_SRVID_SAMBA_NOTIFY message.
+ * ==> retry
+ */
}
+
done:
if (!NT_STATUS_IS_OK(status)) {
--
SAMBA-CTDB repository
More information about the samba-cvs
mailing list