[tarantool-patches] [PATCH 0/5] swim lua preparation

[tarantool-patches] [PATCH 0/5] swim lua preparation

[Vladislav Shpilevoy]

The patchset mainly consists of bug fixes appeared when real libev started to
work instead of the fake event loop used in the unit tests.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-3234-swim-lua-preparation
Issue: https://github.com/tarantool/tarantool/issues/3234

Vladislav Shpilevoy (5):
  swim: do not use ev_timer_start
  swim: introduce member reference API
  sio: return 'no host' flag from sio_uri_to_addr()
  swim: allow to omit host in URI
  swim: explicitly stop old ev_io input/output on rebind

 src/lib/core/sio.c            |  3 +-
 src/lib/core/sio.h            |  2 +-
 src/lib/swim/swim.c           | 69 +++++++++++++++++++++++------------
 src/lib/swim/swim.h           | 24 ++++++++----
 src/lib/swim/swim_constants.h |  5 +++
 src/lib/swim/swim_ev.c        |  6 +++
 src/lib/swim/swim_ev.h        |  3 ++
 src/lib/swim/swim_io.c        | 19 ++++++----
 src/lib/swim/swim_proto.c     |  2 +-
 test/unit/sio.c               | 29 ++++++++++-----
 test/unit/sio.result          | 32 +++++++++-------
 test/unit/swim.c              | 13 ++++++-
 test/unit/swim.result         |  3 +-
 test/unit/swim_test_ev.c      | 11 ++++++
 14 files changed, 154 insertions(+), 67 deletions(-)

-- 
2.20.1 (Apple Git-117)

[tarantool-patches] [PATCH 1/5] swim: do not use ev_timer_start

[Vladislav Shpilevoy]

Appeared that libev changes 'ev_timer.at' field to a remaining
time value, and it can't be used as a storage for a timeout. By
the same reason ev_timer_start() can't be used to reuse a timer.

On the contrary, 'ev_timer.repeat' is not touched by libev, and
ev_timer_again() allows to reuse a timer.

This patch replaces 'at' with 'repeat' and ev_timer_start() with
ev_timer_again().

The bug was not detected by unit tests, because they implement
their own event loop and do not change ev_timer.at. Now they do
to prevent a regression.

Part of #3234
---
 src/lib/swim/swim.c      | 28 +++++++++++-----------------
 src/lib/swim/swim.h      |  4 ----
 src/lib/swim/swim_ev.c   |  6 ++++++
 src/lib/swim/swim_ev.h   |  3 +++
 test/unit/swim_test_ev.c | 11 +++++++++++
 5 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 68b87b120..dfee16493 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -496,7 +496,7 @@ swim_wait_ack(struct swim *swim, struct swim_member *member,
 	      bool was_ping_indirect)
 {
 	if (heap_node_is_stray(&member->in_wait_ack_heap)) {
-		double timeout = swim->wait_ack_tick.at;
+		double timeout = swim->wait_ack_tick.repeat;
 		/*
 		 * Direct ping is two trips: PING + ACK.
 		 * Indirect ping is four trips: PING,
@@ -507,7 +507,7 @@ swim_wait_ack(struct swim *swim, struct swim_member *member,
 			timeout *= 2;
 		member->ping_deadline = swim_time() + timeout;
 		wait_ack_heap_insert(&swim->wait_ack_heap, member);
-		swim_ev_timer_start(loop(), &swim->wait_ack_tick);
+		swim_ev_timer_again(loop(), &swim->wait_ack_tick);
 	}
 }
 
@@ -767,7 +767,7 @@ swim_new_member(struct swim *swim, const struct sockaddr_in *addr,
 		return NULL;
 	}
 	if (mh_size(swim->members) > 1)
-		swim_ev_timer_start(loop(), &swim->round_tick);
+		swim_ev_timer_again(loop(), &swim->round_tick);
 
 	/* Dissemination component. */
 	swim_on_member_update(swim, member);
@@ -1072,7 +1072,7 @@ swim_complete_step(struct swim_task *task,
 	(void) rc;
 	(void) task;
 	struct swim *swim = swim_by_scheduler(scheduler);
-	swim_ev_timer_start(loop(), &swim->round_tick);
+	swim_ev_timer_again(loop(), &swim->round_tick);
 	/*
 	 * It is possible that the original member was deleted
 	 * manually during the task execution.
@@ -1239,7 +1239,7 @@ swim_check_acks(struct ev_loop *loop, struct ev_timer *t, int events)
 	struct swim_member *m;
 	while ((m = wait_ack_heap_top(&swim->wait_ack_heap)) != NULL) {
 		if (current_time < m->ping_deadline) {
-			swim_ev_timer_start(loop, t);
+			swim_ev_timer_again(loop, t);
 			return;
 		}
 		wait_ack_heap_pop(&swim->wait_ack_heap);
@@ -1635,7 +1635,7 @@ swim_new(void)
 	}
 	rlist_create(&swim->round_queue);
 	swim_ev_timer_init(&swim->round_tick, swim_begin_step,
-			   HEARTBEAT_RATE_DEFAULT, 0);
+			   0, HEARTBEAT_RATE_DEFAULT);
 	swim->round_tick.data = (void *) swim;
 	swim_task_create(&swim->round_step_task, swim_complete_step, NULL,
 			 "round packet");
@@ -1644,7 +1644,7 @@ swim_new(void)
 	/* Failure detection component. */
 	wait_ack_heap_create(&swim->wait_ack_heap);
 	swim_ev_timer_init(&swim->wait_ack_tick, swim_check_acks,
-			   ACK_TIMEOUT_DEFAULT, 0);
+			   0, ACK_TIMEOUT_DEFAULT);
 	swim->wait_ack_tick.data = (void *) swim;
 	swim->gc_mode = SWIM_GC_ON;
 
@@ -1737,11 +1737,11 @@ swim_cfg(struct swim *swim, const char *uri, double heartbeat_rate,
 	} else {
 		addr = swim->self->addr;
 	}
-	if (swim->round_tick.at != heartbeat_rate && heartbeat_rate > 0)
-		swim_ev_timer_set(&swim->round_tick, heartbeat_rate, 0);
+	if (swim->round_tick.repeat != heartbeat_rate && heartbeat_rate > 0)
+		swim_ev_timer_set(&swim->round_tick, 0, heartbeat_rate);
 
-	if (swim->wait_ack_tick.at != ack_timeout && ack_timeout > 0)
-		swim_ev_timer_set(&swim->wait_ack_tick, ack_timeout, 0);
+	if (swim->wait_ack_tick.repeat != ack_timeout && ack_timeout > 0)
+		swim_ev_timer_set(&swim->wait_ack_tick, 0, ack_timeout);
 
 	if (new_self != NULL) {
 		swim->self->status = MEMBER_LEFT;
@@ -1763,12 +1763,6 @@ swim_set_codec(struct swim *swim, enum crypto_algo algo, const char *key)
 	return swim_scheduler_set_codec(&swim->scheduler, algo, key);
 }
 
-double
-swim_ack_timeout(const struct swim *swim)
-{
-	return swim->wait_ack_tick.at;
-}
-
 bool
 swim_is_configured(const struct swim *swim)
 {
diff --git a/src/lib/swim/swim.h b/src/lib/swim/swim.h
index 5abbe8c33..653e45be7 100644
--- a/src/lib/swim/swim.h
+++ b/src/lib/swim/swim.h
@@ -101,10 +101,6 @@ swim_cfg(struct swim *swim, const char *uri, double heartbeat_rate,
 	 double ack_timeout, enum swim_gc_mode gc_mode,
 	 const struct tt_uuid *uuid);
 
-/** SWIM's ACK timeout, previously set via @sa swim_cfg. */
-double
-swim_ack_timeout(const struct swim *swim);
-
 /** Set payload to disseminate over the cluster. */
 int
 swim_set_payload(struct swim *swim, const char *payload, uint16_t payload_size);
diff --git a/src/lib/swim/swim_ev.c b/src/lib/swim/swim_ev.c
index f7c464426..49c8c273b 100644
--- a/src/lib/swim/swim_ev.c
+++ b/src/lib/swim/swim_ev.c
@@ -44,6 +44,12 @@ swim_ev_timer_start(struct ev_loop *loop, struct ev_timer *watcher)
 	ev_timer_start(loop, watcher);
 }
 
+void
+swim_ev_timer_again(struct ev_loop *loop, struct ev_timer *watcher)
+{
+	ev_timer_again(loop, watcher);
+}
+
 void
 swim_ev_timer_stop(struct ev_loop *loop, struct ev_timer *watcher)
 {
diff --git a/src/lib/swim/swim_ev.h b/src/lib/swim/swim_ev.h
index 34394ef47..0fe550523 100644
--- a/src/lib/swim/swim_ev.h
+++ b/src/lib/swim/swim_ev.h
@@ -46,6 +46,9 @@ swim_time(void);
 void
 swim_ev_timer_start(struct ev_loop *loop, struct ev_timer *watcher);
 
+void
+swim_ev_timer_again(struct ev_loop *loop, struct ev_timer *watcher);
+
 void
 swim_ev_timer_stop(struct ev_loop *loop, struct ev_timer *watcher);
 
diff --git a/test/unit/swim_test_ev.c b/test/unit/swim_test_ev.c
index 135f20107..16fd87c78 100644
--- a/test/unit/swim_test_ev.c
+++ b/test/unit/swim_test_ev.c
@@ -184,6 +184,7 @@ swim_timer_event_process(struct swim_event *e, struct ev_loop *loop)
 	assert(e->type == SWIM_EVENT_TIMER);
 	struct ev_watcher *w = ((struct swim_timer_event *) e)->watcher;
 	swim_timer_event_delete(e);
+	((struct ev_timer *) w)->at = 0;
 	ev_invoke(loop, w, EV_TIMER);
 }
 
@@ -265,6 +266,16 @@ swim_ev_timer_start(struct ev_loop *loop, struct ev_timer *base)
 	swim_timer_event_new((struct ev_watcher *) base, base->at);
 }
 
+void
+swim_ev_timer_again(struct ev_loop *loop, struct ev_timer *base)
+{
+	(void) loop;
+	if (swim_event_by_ev((struct ev_watcher *) base) != NULL)
+		return;
+	/* Create the periodic watcher and one event. */
+	swim_timer_event_new((struct ev_watcher *) base, base->repeat);
+}
+
 /** Time stop cancels the event if the timer is active. */
 void
 swim_ev_timer_stop(struct ev_loop *loop, struct ev_timer *base)
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 1/5] swim: do not use ev_timer_start

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/01 08:07]:
> Appeared that libev changes 'ev_timer.at' field to a remaining
> time value, and it can't be used as a storage for a timeout. By
> the same reason ev_timer_start() can't be used to reuse a timer.
> 
> On the contrary, 'ev_timer.repeat' is not touched by libev, and
> ev_timer_again() allows to reuse a timer.
> 
> This patch replaces 'at' with 'repeat' and ev_timer_start() with
> ev_timer_again().
> 
> The bug was not detected by unit tests, because they implement
> their own event loop and do not change ev_timer.at. Now they do
> to prevent a regression.

OK to push.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] [PATCH 2/5] swim: introduce member reference API

[Vladislav Shpilevoy]

Struct swim_member pointer is used to learn member's status,
payload, incarnation etc. To obtain a pointer, a one should
either lookup the member by UUID, or obtain from iterators API.
The former is long, the latter is useless when a point lookup is
needed.

On the other hand, it was not safe to keep struct swim_member
pointer for a long time, because it could be deleted at any
moment.

This patch allows to reference a member and be sure that it will
not be deleted until dereferenced explicitly. The member still
can be dropped from the member table, but its memory will be
valid. To detect that a member is dropped, its status is set to
MEMBER_ORPHAN.

Part of #3234
---
 src/lib/swim/swim.c           | 33 ++++++++++++++++++++++++++++-----
 src/lib/swim/swim.h           | 20 +++++++++++++++++---
 src/lib/swim/swim_constants.h |  5 +++++
 src/lib/swim/swim_proto.c     |  2 +-
 test/unit/swim.c              | 13 ++++++++++++-
 test/unit/swim.result         |  3 ++-
 6 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index dfee16493..941214289 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -243,6 +243,12 @@ struct swim_member {
 	 * Position in a queue of members in the current round.
 	 */
 	struct rlist in_round_queue;
+	/**
+	 * Reference counter. Used by public API to prevent the
+	 * member deletion after it is obtained by UUID or from an
+	 * iterator.
+	 */
+	int refs;
 	/**
 	 *
 	 *                 Dissemination component
@@ -638,11 +644,28 @@ swim_ping_task_complete(struct swim_task *task,
 	swim_wait_ack(swim, m, false);
 }
 
+void
+swim_member_ref(struct swim_member *member)
+{
+	++member->refs;
+}
+
+void
+swim_member_unref(struct swim_member *member)
+{
+	assert(member->refs > 0);
+	if (--member->refs == 0) {
+		free(member->payload);
+		free(member);
+	}
+}
+
 /** Free member's resources. */
 static inline void
 swim_member_delete(struct swim_member *member)
 {
 	assert(rlist_empty(&member->in_round_queue));
+	member->status = MEMBER_ORPHAN;
 
 	/* Failure detection component. */
 	assert(heap_node_is_stray(&member->in_wait_ack_heap));
@@ -651,9 +674,8 @@ swim_member_delete(struct swim_member *member)
 
 	/* Dissemination component. */
 	assert(rlist_empty(&member->in_dissemination_queue));
-	free(member->payload);
 
-	free(member);
+	swim_member_unref(member);
 }
 
 /** Create a new member. It is not registered anywhere here. */
@@ -667,6 +689,7 @@ swim_member_new(const struct sockaddr_in *addr, const struct tt_uuid *uuid,
 		diag_set(OutOfMemory, sizeof(*member), "calloc", "member");
 		return NULL;
 	}
+	member->refs = 1;
 	member->status = status;
 	member->addr = *addr;
 	member->uuid = *uuid;
@@ -1969,14 +1992,14 @@ swim_quit(struct swim *swim)
 	swim_quit_step_complete(task, &swim->scheduler, 0);
 }
 
-const struct swim_member *
+struct swim_member *
 swim_self(struct swim *swim)
 {
 	assert(swim_is_configured(swim));
 	return swim->self;
 }
 
-const struct swim_member *
+struct swim_member *
 swim_member_by_uuid(struct swim *swim, const struct tt_uuid *uuid)
 {
 	assert(swim_is_configured(swim));
@@ -1997,7 +2020,7 @@ swim_iterator_open(struct swim *swim)
 	return (struct swim_iterator *) swim;
 }
 
-const struct swim_member *
+struct swim_member *
 swim_iterator_next(struct swim_iterator *iterator)
 {
 	struct swim *swim = (struct swim *) iterator;
diff --git a/src/lib/swim/swim.h b/src/lib/swim/swim.h
index 653e45be7..2584081fb 100644
--- a/src/lib/swim/swim.h
+++ b/src/lib/swim/swim.h
@@ -186,7 +186,7 @@ void
 swim_quit(struct swim *swim);
 
 /** Get a SWIM member, describing this instance. */
-const struct swim_member *
+struct swim_member *
 swim_self(struct swim *swim);
 
 /**
@@ -194,7 +194,7 @@ swim_self(struct swim *swim);
  * @retval NULL Not found.
  * @retval not NULL A member.
  */
-const struct swim_member *
+struct swim_member *
 swim_member_by_uuid(struct swim *swim, const struct tt_uuid *uuid);
 
 /** Member's current status. */
@@ -216,7 +216,7 @@ swim_iterator_open(struct swim *swim);
  * @retval NULL EOF.
  * @retval not NULL A valid member.
  */
-const struct swim_member *
+struct swim_member *
 swim_iterator_next(struct swim_iterator *iterator);
 
 /** Close an iterator. */
@@ -239,6 +239,20 @@ swim_member_incarnation(const struct swim_member *member);
 const char *
 swim_member_payload(const struct swim_member *member, uint16_t *size);
 
+/**
+ * Reference a member. The member memory will be valid until unref
+ * is called.
+ */
+void
+swim_member_ref(struct swim_member *member);
+
+/**
+ * Dereference a member. After that it may be deleted and can't be
+ * accessed anymore.
+ */
+void
+swim_member_unref(struct swim_member *member);
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/src/lib/swim/swim_constants.h b/src/lib/swim/swim_constants.h
index 4f8404ce3..fb44abe2a 100644
--- a/src/lib/swim/swim_constants.h
+++ b/src/lib/swim/swim_constants.h
@@ -50,6 +50,11 @@ enum swim_member_status {
 	MEMBER_DEAD,
 	/** The member has voluntary left the cluster. */
 	MEMBER_LEFT,
+	/**
+	 * The member was dropped from the cluster: either by the
+	 * failure detection, or explicitly.
+	 */
+	MEMBER_ORPHAN,
 	swim_member_status_MAX,
 };
 
diff --git a/src/lib/swim/swim_proto.c b/src/lib/swim/swim_proto.c
index 938631e49..50cd84f94 100644
--- a/src/lib/swim/swim_proto.c
+++ b/src/lib/swim/swim_proto.c
@@ -226,7 +226,7 @@ swim_decode_member_key(enum swim_member_key key, const char **pos,
 		if (swim_decode_uint(pos, end, &tmp, prefix,
 				     "member status") != 0)
 			return -1;
-		if (tmp >= swim_member_status_MAX) {
+		if (tmp >= swim_member_status_MAX || tmp == MEMBER_ORPHAN) {
 			diag_set(SwimError, "%s unknown member status", prefix);
 			return -1;
 		}
diff --git a/test/unit/swim.c b/test/unit/swim.c
index 5aaae089d..65d428ed8 100644
--- a/test/unit/swim.c
+++ b/test/unit/swim.c
@@ -176,7 +176,7 @@ swim_test_cfg(void)
 static void
 swim_test_add_remove(void)
 {
-	swim_start_test(13);
+	swim_start_test(14);
 
 	struct swim_cluster *cluster = swim_cluster_new(2);
 	swim_cluster_add_link(cluster, 0, 1);
@@ -232,6 +232,17 @@ swim_test_add_remove(void)
 	swim_cluster_unblock_io(cluster, 0);
 	is(swim_cluster_wait_fullmesh(cluster, 1), 0,
 	   "back in fullmesh after a member removal in the middle of a step");
+	/*
+	 * Check that member removal does not delete a member,
+	 * only unrefs.
+	 */
+	const struct tt_uuid *s1_uuid = swim_member_uuid(swim_self(s1));
+	struct swim_member *s1_view = swim_member_by_uuid(s2, s1_uuid);
+	swim_member_ref(s1_view);
+	swim_remove_member(s2, s1_uuid);
+	is(swim_member_status(s1_view), MEMBER_ORPHAN, "if a referenced "\
+	   "member is dropped, its status becomes 'orphan'");
+	swim_member_unref(s1_view);
 
 	swim_cluster_delete(cluster);
 
diff --git a/test/unit/swim.result b/test/unit/swim.result
index 6e439541a..d24042164 100644
--- a/test/unit/swim.result
+++ b/test/unit/swim.result
@@ -47,7 +47,7 @@ ok 3 - subtests
 ok 4 - subtests
 	*** swim_test_cfg: done ***
 	*** swim_test_add_remove ***
-    1..13
+    1..14
     ok 1 - can not add an existing member
     ok 2 - diag says 'already exists'
     ok 3 - can not add a invalid uri
@@ -61,6 +61,7 @@ ok 4 - subtests
     ok 11 - after removal the cluster is not in fullmesh
     ok 12 - but it is back in 1 step
     ok 13 - back in fullmesh after a member removal in the middle of a step
+    ok 14 - if a referenced member is dropped, its status becomes 'orphan'
 ok 5 - subtests
 	*** swim_test_add_remove: done ***
 	*** swim_test_basic_failure_detection ***
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 2/5] swim: introduce member reference API

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/01 08:07]:

I disagree with adding a member status for dropped member.

By resetting the status you lose the information about the
original status, as set when leaving the membership table.
Anyway, member status is part of the swim protocol, and should not
be used to reflect any memory management details specific to the
programming language in use.

Please either have a separate status for zombie members 
or simply check for refs == 1.

Re the word itself: orphan means someone who lost a parent. A
member which was kicked out of the membership hasn't necessarily 
lost a parent.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] Re: [PATCH 2/5] swim: introduce member reference API

[Vladislav Shpilevoy]

On 01/05/2019 08:15, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/01 08:07]:
> 
> I disagree with adding a member status for dropped member.
> 
> By resetting the status you lose the information about the
> original status, as set when leaving the membership table.

My motivation of resetting the status was that a user can't
rely on that anymore, it is not valid. If a dropped member's
status is ALIVE, it does not mean, that it is really alive.
User's application should not use the old status.

And I wanted to force a user to stop looking at that invalid
member. Also new status allows to do not add a new function to
check member validity.

Please, reconsider this implementation again after my explanation
above. In case I did not persuade you, I implemented a
flag-based patch, see at the end of the mail.

> Anyway, member status is part of the swim protocol, and should not
> be used to reflect any memory management details specific to the
> programming language in use.
> 
> Please either have a separate status for zombie members 
> or simply check for refs == 1.

I thought about that, refs == 1 does not work. Consider this:
I do swim:find_member() 3 times in Lua. Now ref count of the
member is 4. Then the member is dropped. Its ref count == 3,
but it is dropped. So refs == 1 does not work as a flag.

I added a flag. But please, think again about status invalidation -
it can prevent fallacious belief in status validity in user space.

===================================================================

commit 0040b1966de9ca669676420a5e6c86aacd2f001e
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Tue Apr 30 18:46:58 2019 +0300

    swim: introduce member reference API
    
    Struct swim_member pointer is used to learn member's status,
    payload, incarnation etc. To obtain a pointer, a one should
    either lookup the member by UUID, or obtain from iterators API.
    The former is long, the latter is useless when a point lookup is
    needed.
    
    On the other hand, it was not safe to keep struct swim_member
    pointer for a long time, because it could be deleted at any
    moment.
    
    This patch allows to reference a member and be sure that it will
    not be deleted until dereferenced explicitly. The member still
    can be dropped from the member table, but its memory will be
    valid. To detect that a member is dropped, a user can use
    swim_member_is_dropped() function.
    
    Part of #3234

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index dfee16493..ff95a8c2b 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -243,6 +243,19 @@ struct swim_member {
 	 * Position in a queue of members in the current round.
 	 */
 	struct rlist in_round_queue;
+	/**
+	 * Reference counter. Used by public API to prevent the
+	 * member deletion after it is obtained by UUID or from an
+	 * iterator.
+	 */
+	int refs;
+	/**
+	 * True, if the member was dropped from the member table.
+	 * At the same time it still can be not deleted, if users
+	 * of the public API referenced the member. Dropped member
+	 * is not valid anymore and should be dereferenced.
+	 */
+	bool is_dropped;
 	/**
 	 *
 	 *                 Dissemination component
@@ -638,11 +651,34 @@ swim_ping_task_complete(struct swim_task *task,
 	swim_wait_ack(swim, m, false);
 }
 
+void
+swim_member_ref(struct swim_member *member)
+{
+	++member->refs;
+}
+
+void
+swim_member_unref(struct swim_member *member)
+{
+	assert(member->refs > 0);
+	if (--member->refs == 0) {
+		free(member->payload);
+		free(member);
+	}
+}
+
+bool
+swim_member_is_dropped(const struct swim_member *member)
+{
+	return member->is_dropped;
+}
+
 /** Free member's resources. */
 static inline void
 swim_member_delete(struct swim_member *member)
 {
 	assert(rlist_empty(&member->in_round_queue));
+	member->is_dropped = true;
 
 	/* Failure detection component. */
 	assert(heap_node_is_stray(&member->in_wait_ack_heap));
@@ -651,9 +687,8 @@ swim_member_delete(struct swim_member *member)
 
 	/* Dissemination component. */
 	assert(rlist_empty(&member->in_dissemination_queue));
-	free(member->payload);
 
-	free(member);
+	swim_member_unref(member);
 }
 
 /** Create a new member. It is not registered anywhere here. */
@@ -667,6 +702,7 @@ swim_member_new(const struct sockaddr_in *addr, const struct tt_uuid *uuid,
 		diag_set(OutOfMemory, sizeof(*member), "calloc", "member");
 		return NULL;
 	}
+	member->refs = 1;
 	member->status = status;
 	member->addr = *addr;
 	member->uuid = *uuid;
@@ -1969,14 +2005,14 @@ swim_quit(struct swim *swim)
 	swim_quit_step_complete(task, &swim->scheduler, 0);
 }
 
-const struct swim_member *
+struct swim_member *
 swim_self(struct swim *swim)
 {
 	assert(swim_is_configured(swim));
 	return swim->self;
 }
 
-const struct swim_member *
+struct swim_member *
 swim_member_by_uuid(struct swim *swim, const struct tt_uuid *uuid)
 {
 	assert(swim_is_configured(swim));
@@ -1997,7 +2033,7 @@ swim_iterator_open(struct swim *swim)
 	return (struct swim_iterator *) swim;
 }
 
-const struct swim_member *
+struct swim_member *
 swim_iterator_next(struct swim_iterator *iterator)
 {
 	struct swim *swim = (struct swim *) iterator;
diff --git a/src/lib/swim/swim.h b/src/lib/swim/swim.h
index 653e45be7..2f53716c7 100644
--- a/src/lib/swim/swim.h
+++ b/src/lib/swim/swim.h
@@ -186,7 +186,7 @@ void
 swim_quit(struct swim *swim);
 
 /** Get a SWIM member, describing this instance. */
-const struct swim_member *
+struct swim_member *
 swim_self(struct swim *swim);
 
 /**
@@ -194,7 +194,7 @@ swim_self(struct swim *swim);
  * @retval NULL Not found.
  * @retval not NULL A member.
  */
-const struct swim_member *
+struct swim_member *
 swim_member_by_uuid(struct swim *swim, const struct tt_uuid *uuid);
 
 /** Member's current status. */
@@ -216,7 +216,7 @@ swim_iterator_open(struct swim *swim);
  * @retval NULL EOF.
  * @retval not NULL A valid member.
  */
-const struct swim_member *
+struct swim_member *
 swim_iterator_next(struct swim_iterator *iterator);
 
 /** Close an iterator. */
@@ -239,6 +239,28 @@ swim_member_incarnation(const struct swim_member *member);
 const char *
 swim_member_payload(const struct swim_member *member, uint16_t *size);
 
+/**
+ * Reference a member. The member memory will be valid until unref
+ * is called.
+ */
+void
+swim_member_ref(struct swim_member *member);
+
+/**
+ * Dereference a member. After that it may be deleted and can't be
+ * accessed anymore.
+ */
+void
+swim_member_unref(struct swim_member *member);
+
+/**
+ * Check if a member was dropped from the member table. It means,
+ * that the member is not valid anymore and should be
+ * dereferenced.
+ */
+bool
+swim_member_is_dropped(const struct swim_member *member);
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/test/unit/swim.c b/test/unit/swim.c
index 5aaae089d..8303e2900 100644
--- a/test/unit/swim.c
+++ b/test/unit/swim.c
@@ -176,7 +176,7 @@ swim_test_cfg(void)
 static void
 swim_test_add_remove(void)
 {
-	swim_start_test(13);
+	swim_start_test(14);
 
 	struct swim_cluster *cluster = swim_cluster_new(2);
 	swim_cluster_add_link(cluster, 0, 1);
@@ -232,6 +232,17 @@ swim_test_add_remove(void)
 	swim_cluster_unblock_io(cluster, 0);
 	is(swim_cluster_wait_fullmesh(cluster, 1), 0,
 	   "back in fullmesh after a member removal in the middle of a step");
+	/*
+	 * Check that member removal does not delete a member,
+	 * only unrefs.
+	 */
+	const struct tt_uuid *s1_uuid = swim_member_uuid(swim_self(s1));
+	struct swim_member *s1_view = swim_member_by_uuid(s2, s1_uuid);
+	swim_member_ref(s1_view);
+	swim_remove_member(s2, s1_uuid);
+	ok(swim_member_is_dropped(s1_view), "if a referenced "\
+	   "member is dropped, it can be detected from the public API");
+	swim_member_unref(s1_view);
 
 	swim_cluster_delete(cluster);
 
diff --git a/test/unit/swim.result b/test/unit/swim.result
index 6e439541a..24597641c 100644
--- a/test/unit/swim.result
+++ b/test/unit/swim.result
@@ -47,7 +47,7 @@ ok 3 - subtests
 ok 4 - subtests
 	*** swim_test_cfg: done ***
 	*** swim_test_add_remove ***
-    1..13
+    1..14
     ok 1 - can not add an existing member
     ok 2 - diag says 'already exists'
     ok 3 - can not add a invalid uri
@@ -61,6 +61,7 @@ ok 4 - subtests
     ok 11 - after removal the cluster is not in fullmesh
     ok 12 - but it is back in 1 step
     ok 13 - back in fullmesh after a member removal in the middle of a step
+    ok 14 - if a referenced member is dropped, it can be detected from the public API
 ok 5 - subtests
 	*** swim_test_add_remove: done ***
 	*** swim_test_basic_failure_detection ***

[tarantool-patches] Re: [PATCH 2/5] swim: introduce member reference API

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/02 18:11]:

I still think a flag is better. Even if I can't rely on member
status, I still may want to know what it was. You could ask Vova
for a second opinion.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] Re: [PATCH 2/5] swim: introduce member reference API

[Vladislav Shpilevoy]

On 02/05/2019 18:46, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/02 18:11]:
> 
> I still think a flag is better. Even if I can't rely on member
> status, I still may want to know what it was.

Ok, probably you are right. Like a postmortem.

> You could ask Vova
> for a second opinion.

Vova does not care about SWIM, we've discussed
that two times already.

> 
> 
> -- 
> Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
> 

[tarantool-patches] [PATCH 3/5] sio: return 'no host' flag from sio_uri_to_addr()

[Vladislav Shpilevoy]

Results of sio_uri_to_addr("0.0.0.0:port") and
sio_uri_to_addr("port") are indistinguishable - both fill
sockaddr_in.sin_addr as INADDR_ANY. But SWIM wants to forbid the
former, and allow the latter. In case if only port is specified,
host will be 'localhost'.

This commit returns a flag from sio_uri_to_addr() signaling if a
host was specified.
---
 src/lib/core/sio.c   |  3 ++-
 src/lib/core/sio.h   |  2 +-
 src/lib/swim/swim.c  |  4 +++-
 test/unit/sio.c      | 29 ++++++++++++++++++++---------
 test/unit/sio.result | 32 ++++++++++++++++++--------------
 5 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/src/lib/core/sio.c b/src/lib/core/sio.c
index 28aef1d6c..97a512eee 100644
--- a/src/lib/core/sio.c
+++ b/src/lib/core/sio.c
@@ -344,11 +344,12 @@ sio_strfaddr(const struct sockaddr *addr, socklen_t addrlen)
 }
 
 int
-sio_uri_to_addr(const char *uri, struct sockaddr *addr)
+sio_uri_to_addr(const char *uri, struct sockaddr *addr, bool *is_host_empty)
 {
 	struct uri u;
 	if (uri_parse(&u, uri) != 0 || u.service == NULL)
 		goto invalid_uri;
+	*is_host_empty = u.host_len == 0;
 	if (u.host_len == strlen(URI_HOST_UNIX) &&
 	    memcmp(u.host, URI_HOST_UNIX, u.host_len) == 0) {
 		struct sockaddr_un *un = (struct sockaddr_un *) addr;
diff --git a/src/lib/core/sio.h b/src/lib/core/sio.h
index 093b5f5ba..db2e3281f 100644
--- a/src/lib/core/sio.h
+++ b/src/lib/core/sio.h
@@ -223,7 +223,7 @@ ssize_t sio_recvfrom(int fd, void *buf, size_t len, int flags,
  * sockaddr_in/un structure.
  */
 int
-sio_uri_to_addr(const char *uri, struct sockaddr *addr);
+sio_uri_to_addr(const char *uri, struct sockaddr *addr, bool *is_host_empty);
 
 #if defined(__cplusplus)
 } /* extern "C" */
diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 941214289..22dfc3d2a 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -1686,7 +1686,9 @@ swim_uri_to_addr(const char *uri, struct sockaddr_in *addr,
 		 const char *prefix)
 {
 	struct sockaddr_storage storage;
-	if (sio_uri_to_addr(uri, (struct sockaddr *) &storage) != 0)
+	bool is_host_empty;
+	if (sio_uri_to_addr(uri, (struct sockaddr *) &storage,
+			    &is_host_empty) != 0)
 		return -1;
 	if (storage.ss_family != AF_INET) {
 		diag_set(IllegalParams, "%s only IP sockets are supported",
diff --git a/test/unit/sio.c b/test/unit/sio.c
index dffdd5592..4ded96f4a 100644
--- a/test/unit/sio.c
+++ b/test/unit/sio.c
@@ -39,40 +39,49 @@ static void
 check_uri_to_addr(void)
 {
 	header();
-	plan(18);
+	plan(22);
+	bool is_host_empty;
 	struct sockaddr_storage storage;
 	struct sockaddr *addr = (struct sockaddr *) &storage;
 	struct sockaddr_un *un = (struct sockaddr_un *) addr;
 	struct sockaddr_in *in = (struct sockaddr_in *) addr;
-	isnt(0, sio_uri_to_addr("invalid uri", addr),
+	isnt(0, sio_uri_to_addr("invalid uri", addr, &is_host_empty),
 	     "invalid uri is detected");
 
 	char long_path[1000];
 	char *pos = long_path + sprintf(long_path, "unix/:/");
 	memset(pos, 'a', 900);
 	pos[900] = 0;
-	isnt(0, sio_uri_to_addr(long_path, addr), "too long UNIX path");
+	isnt(0, sio_uri_to_addr(long_path, addr, &is_host_empty),
+	     "too long UNIX path");
 
-	is(0, sio_uri_to_addr("unix/:/normal_path", addr), "UNIX");
+	is(0, sio_uri_to_addr("unix/:/normal_path", addr, &is_host_empty),
+	   "UNIX");
 	is(0, strcmp(un->sun_path, "/normal_path"), "UNIX path");
 	is(AF_UNIX, un->sun_family, "UNIX family");
+	ok(! is_host_empty, "unix host is not empty");
 
-	is(0, sio_uri_to_addr("localhost:1234", addr), "localhost");
+	is(0, sio_uri_to_addr("localhost:1234", addr, &is_host_empty),
+	   "localhost");
 	is(AF_INET, in->sin_family, "localhost family");
 	is(htonl(INADDR_LOOPBACK), in->sin_addr.s_addr, "localhost address");
 	is(htons(1234), in->sin_port, "localhost port");
+	ok(! is_host_empty, "'localhost' host is not empty");
 
-	is(0, sio_uri_to_addr("5678", addr), "'any'");
+	is(0, sio_uri_to_addr("5678", addr, &is_host_empty), "'any'");
 	is(AF_INET, in->sin_family, "'any' family");
 	is(htonl(INADDR_ANY), in->sin_addr.s_addr, "'any' address");
 	is(htons(5678), in->sin_port, "'any' port");
+	ok(is_host_empty, "only port specified - host is empty");
 
-	is(0, sio_uri_to_addr("192.168.0.1:9101", addr), "IP");
+	is(0, sio_uri_to_addr("192.168.0.1:9101", addr, &is_host_empty), "IP");
 	is(AF_INET, in->sin_family, "IP family");
 	is(inet_addr("192.168.0.1"), in->sin_addr.s_addr, "IP address");
 	is(htons(9101), in->sin_port, "IP port");
+	ok(! is_host_empty, "IPv4 host is not empty");
 
-	isnt(0, sio_uri_to_addr("192.168.0.300:1112", addr), "invalid IP");
+	isnt(0, sio_uri_to_addr("192.168.0.300:1112", addr, &is_host_empty),
+	     "invalid IP");
 
 	check_plan();
 	footer();
@@ -84,9 +93,11 @@ check_auto_bind(void)
 	header();
 	plan(3);
 
+	bool is_host_empty;
 	struct sockaddr_in addr;
 	socklen_t addrlen = sizeof(addr);
-	sio_uri_to_addr("127.0.0.1:0", (struct sockaddr *) &addr);
+	sio_uri_to_addr("127.0.0.1:0", (struct sockaddr *) &addr,
+			&is_host_empty);
 	int fd = sio_socket(AF_INET, SOCK_STREAM, 0);
 	is(sio_bind(fd, (struct sockaddr *) &addr, sizeof(addr)), 0,
 	   "bind to 0 works");
diff --git a/test/unit/sio.result b/test/unit/sio.result
index e5712ad3c..896736bf9 100644
--- a/test/unit/sio.result
+++ b/test/unit/sio.result
@@ -1,25 +1,29 @@
 	*** main ***
 1..2
 	*** check_uri_to_addr ***
-    1..18
+    1..22
     ok 1 - invalid uri is detected
     ok 2 - too long UNIX path
     ok 3 - UNIX
     ok 4 - UNIX path
     ok 5 - UNIX family
-    ok 6 - localhost
-    ok 7 - localhost family
-    ok 8 - localhost address
-    ok 9 - localhost port
-    ok 10 - 'any'
-    ok 11 - 'any' family
-    ok 12 - 'any' address
-    ok 13 - 'any' port
-    ok 14 - IP
-    ok 15 - IP family
-    ok 16 - IP address
-    ok 17 - IP port
-    ok 18 - invalid IP
+    ok 6 - unix host is not empty
+    ok 7 - localhost
+    ok 8 - localhost family
+    ok 9 - localhost address
+    ok 10 - localhost port
+    ok 11 - 'localhost' host is not empty
+    ok 12 - 'any'
+    ok 13 - 'any' family
+    ok 14 - 'any' address
+    ok 15 - 'any' port
+    ok 16 - only port specified - host is empty
+    ok 17 - IP
+    ok 18 - IP family
+    ok 19 - IP address
+    ok 20 - IP port
+    ok 21 - IPv4 host is not empty
+    ok 22 - invalid IP
 ok 1 - subtests
 	*** check_uri_to_addr: done ***
 	*** check_auto_bind ***
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 3/5] sio: return 'no host' flag from sio_uri_to_addr()

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/01 08:07]:
> Results of sio_uri_to_addr("0.0.0.0:port") and
> sio_uri_to_addr("port") are indistinguishable - both fill
> sockaddr_in.sin_addr as INADDR_ANY. But SWIM wants to forbid the
> former, and allow the latter. In case if only port is specified,
> host will be 'localhost'.
> 
> This commit returns a flag from sio_uri_to_addr() signaling if a
> host was specified.

I don't see how you're using the return value. What am I not
understanding?


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] [PATCH 4/5] swim: allow to omit host in URI

[Vladislav Shpilevoy]

Before the patch swim.cfg() was returning an error on an attempt
to use URI like 'port', without a host. But it would be useful,
easy, and short to allow that, and use '127.0.0.1' host by
default. Compare:

    swim:cfg({uri = 1234})
            vs
    swim:cfg({uri = '127.0.0.1:1234'})

It is remarkable that box.cfg{listen} also allows to omit host.

Note, that use '0.0.0.0' (INADDR_ANY) is forbidden. Otherwise

    1) Different instances interacting with this one via not the
       same interfaces would see different source IP addresses.
       It would mess member tables;

    2) This instance would not be able to encode its IP address
       in the meta section, because it has no a fixed IP. At the
       same time omission of it and usage of UDP header source
       address is not possible as well, because UDP header is not
       encrypted and therefore is not safe to look at.

Part of #3234
---
 src/lib/swim/swim.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 22dfc3d2a..b2b4901b1 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -1696,7 +1696,9 @@ swim_uri_to_addr(const char *uri, struct sockaddr_in *addr,
 		return -1;
 	}
 	*addr = *((struct sockaddr_in *) &storage);
-	if (addr->sin_addr.s_addr == 0) {
+	if (is_host_empty) {
+		addr->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+	} else if (addr->sin_addr.s_addr == 0) {
 		diag_set(IllegalParams, "%s INADDR_ANY is not supported",
 			 prefix);
 		return -1;
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 4/5] swim: allow to omit host in URI

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/01 08:07]:
> Before the patch swim.cfg() was returning an error on an attempt
> to use URI like 'port', without a host. But it would be useful,
> easy, and short to allow that, and use '127.0.0.1' host by
> default. Compare:
> 
>     swim:cfg({uri = 1234})
>             vs
>     swim:cfg({uri = '127.0.0.1:1234'})
> 
> It is remarkable that box.cfg{listen} also allows to omit host.
> 
> Note, that use '0.0.0.0' (INADDR_ANY) is forbidden. Otherwise
> 
>     1) Different instances interacting with this one via not the
>        same interfaces would see different source IP addresses.
>        It would mess member tables;
> 
>     2) This instance would not be able to encode its IP address
>        in the meta section, because it has no a fixed IP. At the
>        same time omission of it and usage of UDP header source
>        address is not possible as well, because UDP header is not
>        encrypted and therefore is not safe to look at.
> 

Thank you for an elaborate description. Please squash the patch
with the previous one. I believe at least some of this extensive
comment belongs to the source code. The patch is OK to push after
adding a comment.

> Part of #3234
> ---
>  src/lib/swim/swim.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
> index 22dfc3d2a..b2b4901b1 100644
> --- a/src/lib/swim/swim.c
> +++ b/src/lib/swim/swim.c
> @@ -1696,7 +1696,9 @@ swim_uri_to_addr(const char *uri, struct sockaddr_in *addr,
>  		return -1;
>  	}
>  	*addr = *((struct sockaddr_in *) &storage);
> -	if (addr->sin_addr.s_addr == 0) {
> +	if (is_host_empty) {
> +		addr->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> +	} else if (addr->sin_addr.s_addr == 0) {
>  		diag_set(IllegalParams, "%s INADDR_ANY is not supported",
>  			 prefix);
>  		return -1;
> -- 
> 2.20.1 (Apple Git-117)
> 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] Re: [PATCH 4/5] swim: allow to omit host in URI

[Vladislav Shpilevoy]

On 01/05/2019 08:20, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/01 08:07]:
>> Before the patch swim.cfg() was returning an error on an attempt
>> to use URI like 'port', without a host. But it would be useful,
>> easy, and short to allow that, and use '127.0.0.1' host by
>> default. Compare:
>>
>>     swim:cfg({uri = 1234})
>>             vs
>>     swim:cfg({uri = '127.0.0.1:1234'})
>>
>> It is remarkable that box.cfg{listen} also allows to omit host.
>>
>> Note, that use '0.0.0.0' (INADDR_ANY) is forbidden. Otherwise
>>
>>     1) Different instances interacting with this one via not the
>>        same interfaces would see different source IP addresses.
>>        It would mess member tables;
>>
>>     2) This instance would not be able to encode its IP address
>>        in the meta section, because it has no a fixed IP. At the
>>        same time omission of it and usage of UDP header source
>>        address is not possible as well, because UDP header is not
>>        encrypted and therefore is not safe to look at.
>>
> 
> Thank you for an elaborate description. Please squash the patch
> with the previous one. I believe at least some of this extensive
> comment belongs to the source code. The patch is OK to push after
> adding a comment.
> 

Added:

================================================================
diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index c806000df..ada36156e 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -1710,6 +1710,27 @@ swim_uri_to_addr(const char *uri, struct sockaddr_in *addr,
 	}
 	*addr = *((struct sockaddr_in *) &storage);
 	if (is_host_empty) {
+		/*
+		 * This condition is satisfied when host is
+		 * omitted and URI is "port". Note, that
+		 * traditionally "port" is converted to
+		 * "0.0.0.0:port" what means binding to all the
+		 * interfaces simultaneously, but it would not
+		 * work for SWIM. There is why:
+		 *
+		 *     - Different instances interacting with this
+		 *       one via not the same interface would see
+		 *       different source IP addresses. It would
+		 *       mess member tables;
+		 *
+		 *     - This instance would not be able to encode
+		 *       its IP address in the meta section,
+		 *       because it has no a fixed IP. At the same
+		 *       time omission of it and usage of UDP
+		 *       header source address is not possible as
+		 *       well, because UDP header is not encrypted
+		 *       and therefore is not safe to look at.
+		 */
 		addr->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
 	} else if (addr->sin_addr.s_addr == 0) {
 		diag_set(IllegalParams, "%s INADDR_ANY is not supported",
diff --git a/src/lib/swim/swim.h b/src/lib/swim/swim.h
index 2f53716c7..a3711dc27 100644
--- a/src/lib/swim/swim.h
+++ b/src/lib/swim/swim.h
@@ -77,7 +77,8 @@ swim_is_configured(const struct swim *swim);
  * Configure or reconfigure a SWIM instance.
  *
  * @param swim SWIM instance to configure.
- * @param uri URI in the format "ip:port".
+ * @param uri URI in the format "ip:port", or "port". In the
+ *        latter case host is "127.0.0.1" by default.
  * @param heartbeat_rate Rate of sending round messages. It does
  *        not mean that each member will be checked each
  *        @heartbeat_rate seconds. It is rather the protocol
================================================================

[tarantool-patches] [PATCH 5/5] swim: explicitly stop old ev_io input/output on rebind

[Vladislav Shpilevoy]

When an input/output file descriptor was changed, SWIM did this:

    swim_ev_io_set(io, new_fd)
    swim_ev_io_start(io)

It worked in an assumption that libev allows to change fd on fly,
and the tests were passing because fake event loop was used, not
real libev.

But it didn't work. Libev requires explicit ev_io_stop() called
on the old descriptor. This patch makes this:

    swim_ev_io_stop(io)
    //do bind ...

    swim_ev_io_set(io, new_fd)
    swim_ev_io_start(io)

Part of #3234
---
 src/lib/swim/swim_io.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/lib/swim/swim_io.c b/src/lib/swim/swim_io.c
index d49900a86..ef2f479eb 100644
--- a/src/lib/swim/swim_io.c
+++ b/src/lib/swim/swim_io.c
@@ -288,15 +288,18 @@ int
 swim_scheduler_bind(struct swim_scheduler *scheduler,
 		    const struct sockaddr_in *addr)
 {
+	swim_ev_io_stop(loop(), &scheduler->input);
+	swim_ev_io_stop(loop(), &scheduler->output);
 	struct swim_transport *t = &scheduler->transport;
-	if (swim_transport_bind(t, (const struct sockaddr *) addr,
-				sizeof(*addr)) != 0)
-		return -1;
-	swim_ev_io_set(&scheduler->output, t->fd, EV_WRITE);
-	swim_ev_io_set(&scheduler->input, t->fd, EV_READ);
-	swim_ev_io_start(loop(), &scheduler->input);
-	swim_ev_io_start(loop(), &scheduler->output);
-	return 0;
+	int rc = swim_transport_bind(t, (const struct sockaddr *) addr,
+				     sizeof(*addr));
+	if (t->fd >= 0) {
+		swim_ev_io_set(&scheduler->output, t->fd, EV_WRITE);
+		swim_ev_io_set(&scheduler->input, t->fd, EV_READ);
+		swim_ev_io_start(loop(), &scheduler->input);
+		swim_ev_io_start(loop(), &scheduler->output);
+	}
+	return rc;
 }
 
 void
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 5/5] swim: explicitly stop old ev_io input/output on rebind

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/01 08:07]:
> When an input/output file descriptor was changed, SWIM did this:
> 
>     swim_ev_io_set(io, new_fd)
>     swim_ev_io_start(io)
> 

OK to push.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] Re: [PATCH 0/5] swim lua preparation

[Vladislav Shpilevoy]

The patchset is pushed into the master. Changes:

"sio: return 'no host' flag from sio_uri_to_addr()"

    is squashed with

"swim: allow to omit host in URI".

In the reference patch 'status' is not affected, and
an explicit flag 'is_dropped' is added.

On 01/05/2019 01:31, Vladislav Shpilevoy wrote:
> The patchset mainly consists of bug fixes appeared when real libev started to
> work instead of the fake event loop used in the unit tests.
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-3234-swim-lua-preparation
> Issue: https://github.com/tarantool/tarantool/issues/3234
> 
> Vladislav Shpilevoy (5):
>   swim: do not use ev_timer_start
>   swim: introduce member reference API
>   sio: return 'no host' flag from sio_uri_to_addr()
>   swim: allow to omit host in URI
>   swim: explicitly stop old ev_io input/output on rebind
> 
>  src/lib/core/sio.c            |  3 +-
>  src/lib/core/sio.h            |  2 +-
>  src/lib/swim/swim.c           | 69 +++++++++++++++++++++++------------
>  src/lib/swim/swim.h           | 24 ++++++++----
>  src/lib/swim/swim_constants.h |  5 +++
>  src/lib/swim/swim_ev.c        |  6 +++
>  src/lib/swim/swim_ev.h        |  3 ++
>  src/lib/swim/swim_io.c        | 19 ++++++----
>  src/lib/swim/swim_proto.c     |  2 +-
>  test/unit/sio.c               | 29 ++++++++++-----
>  test/unit/sio.result          | 32 +++++++++-------
>  test/unit/swim.c              | 13 ++++++-
>  test/unit/swim.result         |  3 +-
>  test/unit/swim_test_ev.c      | 11 ++++++
>  14 files changed, 154 insertions(+), 67 deletions(-)
> 
> -- 
> 2.20.1 (Apple Git-117)
> 
>