Date: Mon, 01 Nov 2010 17:06:49 +0200 From: Mikolaj Golub <to.my.trociny@gmail.com> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: freebsd-stable@freebsd.org, Pete French <petefrench@ticketswitch.com> Subject: Re: hast vs ggate+gmirror sychrnoisation speed Message-ID: <86d3qpqe0m.fsf@zhuzha.ua1> In-Reply-To: <20101101110100.GI2160@garage.freebsd.pl> (Pawel Jakub Dawidek's message of "Mon, 1 Nov 2010 12:01:00 %2B0100") References: <E1PAlxN-000H5x-Eh@dilbert.ticketswitch.com> <86wrp3wj67.fsf@kopusha.home.net> <20101028163036.GA2347@garage.freebsd.pl> <86lj5i3zjt.fsf@kopusha.home.net> <86d3qr3m0b.fsf@kopusha.home.net> <20101101110100.GI2160@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
On Mon, 1 Nov 2010 12:01:00 +0100 Pawel Jakub Dawidek wrote:
PJD> I like your patch and I agree of course it is better to send keepalive
PJD> packets only when connection is idle. The only thing I'd change is to
PJD> modify QUEUE_TAKE1() macro to take additional argument 'timeout' - if we
PJD> don't want it to time out, we pass 0. Could you modify your patch?
Sure :-). Could you look at the updated version?
Note. So far I have only tested that hastd with this updated patch is
compilable and runnable. I will do normal testing today later when I have
access to my test instances and will report about the results.
--
Mikolaj Golub
--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=hastd.keepalive.2.patch
Index: sbin/hastd/primary.c
===================================================================
--- sbin/hastd/primary.c (revision 214624)
+++ sbin/hastd/primary.c (working copy)
@@ -180,14 +180,20 @@ static pthread_mutex_t metadata_lock;
if (_wakeup) \
cv_signal(&hio_##name##_list_cond); \
} while (0)
-#define QUEUE_TAKE1(hio, name, ncomp) do { \
+#define QUEUE_TAKE1(hio, name, ncomp, timeout) do { \
+ bool _last; \
+ \
mtx_lock(&hio_##name##_list_lock[(ncomp)]); \
- while (((hio) = TAILQ_FIRST(&hio_##name##_list[(ncomp)])) == NULL) { \
- cv_wait(&hio_##name##_list_cond[(ncomp)], \
- &hio_##name##_list_lock[(ncomp)]); \
+ _last = false; \
+ while (((hio) = TAILQ_FIRST(&hio_##name##_list[(ncomp)])) == NULL && !_last) { \
+ cv_timedwait(&hio_##name##_list_cond[(ncomp)], \
+ &hio_##name##_list_lock[(ncomp)], (timeout)); \
+ if ((timeout) != 0) \
+ _last = true; \
} \
- TAILQ_REMOVE(&hio_##name##_list[(ncomp)], (hio), \
- hio_next[(ncomp)]); \
+ if (hio != NULL) \
+ TAILQ_REMOVE(&hio_##name##_list[(ncomp)], (hio), \
+ hio_next[(ncomp)]); \
mtx_unlock(&hio_##name##_list_lock[(ncomp)]); \
} while (0)
#define QUEUE_TAKE2(hio, name) do { \
@@ -1112,7 +1118,7 @@ local_send_thread(void *arg)
for (;;) {
pjdlog_debug(2, "local_send: Taking request.");
- QUEUE_TAKE1(hio, send, ncomp);
+ QUEUE_TAKE1(hio, send, ncomp, 0);
pjdlog_debug(2, "local_send: (%p) Got request.", hio);
ggio = &hio->hio_ggio;
switch (ggio->gctl_cmd) {
@@ -1176,6 +1182,38 @@ local_send_thread(void *arg)
return (NULL);
}
+static void
+keepalive_send(struct hast_resource *res, unsigned int ncomp)
+{
+ struct nv *nv;
+
+ if (!ISCONNECTED(res, ncomp))
+ return;
+
+ assert(res->hr_remotein != NULL);
+ assert(res->hr_remoteout != NULL);
+
+ nv = nv_alloc();
+ nv_add_uint8(nv, HIO_KEEPALIVE, "cmd");
+ if (nv_error(nv) != 0) {
+ nv_free(nv);
+ pjdlog_debug(1,
+ "keepalive_send: Unable to prepare header to send.");
+ return;
+ }
+ if (hast_proto_send(res, res->hr_remoteout, nv, NULL, 0) < 0) {
+ pjdlog_common(LOG_DEBUG, 1, errno,
+ "keepalive_send: Unable to send request");
+ nv_free(nv);
+ rw_unlock(&hio_remote_lock[ncomp]);
+ remote_close(res, ncomp);
+ rw_rlock(&hio_remote_lock[ncomp]);
+ return;
+ }
+ nv_free(nv);
+ pjdlog_debug(2, "keepalive_send: Request sent.");
+}
+
/*
* Thread sends request to secondary node.
*/
@@ -1184,6 +1222,7 @@ remote_send_thread(void *arg)
{
struct hast_resource *res = arg;
struct g_gate_ctl_io *ggio;
+ time_t lastcheck, now;
struct hio *hio;
struct nv *nv;
unsigned int ncomp;
@@ -1194,10 +1233,19 @@ remote_send_thread(void *arg)
/* Remote component is 1 for now. */
ncomp = 1;
+ lastcheck = time(NULL);
for (;;) {
pjdlog_debug(2, "remote_send: Taking request.");
- QUEUE_TAKE1(hio, send, ncomp);
+ QUEUE_TAKE1(hio, send, ncomp, RETRY_SLEEP);
+ if (hio == NULL) {
+ now = time(NULL);
+ if (lastcheck + RETRY_SLEEP <= now) {
+ keepalive_send(res, ncomp);
+ lastcheck = now;
+ }
+ continue;
+ }
pjdlog_debug(2, "remote_send: (%p) Got request.", hio);
ggio = &hio->hio_ggio;
switch (ggio->gctl_cmd) {
@@ -1883,32 +1931,6 @@ failed:
}
static void
-keepalive_send(struct hast_resource *res, unsigned int ncomp)
-{
- struct nv *nv;
-
- nv = nv_alloc();
- nv_add_uint8(nv, HIO_KEEPALIVE, "cmd");
- if (nv_error(nv) != 0) {
- nv_free(nv);
- pjdlog_debug(1,
- "keepalive_send: Unable to prepare header to send.");
- return;
- }
- if (hast_proto_send(res, res->hr_remoteout, nv, NULL, 0) < 0) {
- pjdlog_common(LOG_DEBUG, 1, errno,
- "keepalive_send: Unable to send request");
- nv_free(nv);
- rw_unlock(&hio_remote_lock[ncomp]);
- remote_close(res, ncomp);
- rw_rlock(&hio_remote_lock[ncomp]);
- return;
- }
- nv_free(nv);
- pjdlog_debug(2, "keepalive_send: Request sent.");
-}
-
-static void
guard_one(struct hast_resource *res, unsigned int ncomp)
{
struct proto_conn *in, *out;
@@ -1926,12 +1948,6 @@ guard_one(struct hast_resource *res, unsigned int
if (ISCONNECTED(res, ncomp)) {
assert(res->hr_remotein != NULL);
assert(res->hr_remoteout != NULL);
- keepalive_send(res, ncomp);
- }
-
- if (ISCONNECTED(res, ncomp)) {
- assert(res->hr_remotein != NULL);
- assert(res->hr_remoteout != NULL);
rw_unlock(&hio_remote_lock[ncomp]);
pjdlog_debug(2, "remote_guard: Connection to %s is ok.",
res->hr_remoteaddr);
--=-=-=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86d3qpqe0m.fsf>