[tarantool-patches] Re: [PATCH 1/2] swim: introduce broadcast tasks

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 11/04/2019 15:57, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/10 21:01]:
>> When a cluster is just created, no one knows anyone. Broadcast
>> helps to establish some initial relationships between members.
>>
>> This commit introduces only an interface to create broadcast
>> tasks from SWIM code. The next commit uses this interface to
>> implement ping broadcast.
> 
>> +/**
>> + * Broadcast task. Besides usual task fields, stores a list of
>> + * interfaces available for broadcast packets. The task works
>> + * multiple times, each time sending a packet to one interface.
>> + * After completion it is self-deleted.
>> + */
>> +struct swim_bcast_task {
> 
>     Please explain in the comment why you need a task - you may
>     only broadcast when a socket is writable, so use a generic way to 
>     interact with the transport. Please mention why you create a
>     single task, not multiple - no particular reason, just to
>     create fewer tasks and do less error checking for failed
>     resources.

My way has nothing to do with less error checking. I still check
for no interfaces, for getifaddrs() error, for IO errors. The only
reason - I wanted not to create many tasks, to save some memory.

>     Please mention in a comment to swim_broadcast that
>     we send a broadcast message to all interfaces which are
>     support IP protocol and broadcast messages. 

I did it in the next commit. Here we do not have swim_broadcast()
yet.


I've update the comment and pushed to the master:

===================================================================
diff --git a/src/lib/swim/swim_io.h b/src/lib/swim/swim_io.h
index 30acd491f..5d0cb0d47 100644
--- a/src/lib/swim/swim_io.h
+++ b/src/lib/swim/swim_io.h
@@ -250,8 +250,16 @@ swim_task_destroy(struct swim_task *task)
 /**
  * Broadcast task. Besides usual task fields, stores a list of
  * interfaces available for broadcast packets. The task works
- * multiple times, each time sending a packet to one interface.
- * After completion it is self-deleted.
+ * asynchronously just like its ancestor, because even broadcast
+ * packets can not be sent without explicit permission from libev
+ * in a form of EV_WRITE event.
+ *
+ * Despite usually having multiple network interfaces supporting
+ * broadcast, there is only one task to send a packet to all of
+ * them. The same task works multiple times, each time sending a
+ * packet to one interface. After completion it is self-deleted.
+ * There are no any concrete reason behind that except stint on
+ * memory for multiple tasks.
  */
 struct swim_bcast_task {
 	/** Base structure. */
===================================================================

> 
> 
> Otherwise the patch is OK to push.
> 
> -- 
> Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
> http://tarantool.io - www.twitter.com/kostja_osipov
>