Re: [systemd-devel] [PATCH] core: support Distribute=n to distribute to n workers

Shawn Landden Fri, 13 Dec 2013 12:31:53 -0800

Forgot to send my notes on the last review....

On Fri, Dec 13, 2013 at 7:12 AM, Lennart Poettering
<lenn...@poettering.net> wrote:
> On Thu, 12.12.13 23:46, Shawn Landden (sh...@churchofgit.com) wrote:
>
>> -Socket.ReusePort,                config_parse_bool,                  0,     
>>                         offsetof(Socket, reuse_port)
>> +Socket.ReusePort,                config_parse_tristate,             -1,     
>>                         offsetof(Socket, reuse_port)
>                                                                       ^^^^^
>
> Why -1 there? That should be there... The parse call doesn't make use of
> that, so it should be 0, really.
fixed, and i see your point, but i still don't know where the default
of -1 comes from...
>
>
>> +        if (s->reuse_port < 0) {
>> +                if (s->distribute > 0)
>> +                        s->reuse_port = true;
>> +                else
>> +                        s->reuse_port = false;
>> +        }
>> +
>
>
> Nitpicking: I'd just write it like this:
>
> if (s->reuse_port < 0)
>         s->reuse_port = s->distribute > 0;
thats better
>
>
>> -                if (s->n_connections >= s->max_connections) {
>> +                if (s->n_connections >= s->max_connections && 
>> !(s->distribute)) {
>
> Still too many ()....
damn
>
>>
>> -        if (se->state == SERVICE_RUNNING)
>> -                socket_set_state(s, SOCKET_RUNNING);
>> +        if (se->state == SERVICE_RUNNING) {
>> +                if (s->n_connections < s->distribute)
>> +                        ;
>> +                else
>> +                        socket_set_state(s, SOCKET_RUNNING);
>> +        }
>
> Hmm, too simple, no? We wanted that logic, that we enter
> SOCKET_RUNNING as long as at least one service is still starting up or
> when we reached the limit. I only see the check for the latter here...
So before I checked for Type=notify in socket_enter_running and then
only spawned one service,
so that I could go back to SOCKET_LISTENING here, but without that
logic, I don't see
a need for any special logic here. Since we only get these
notifications for Type=notify,
we can't just unilaterally go to


>
> Still missing the bit where the socket is duplicated for propery
> SO_REUSEPORT usgae...
OK, so this is the way lwn covered it, but using fork() to replicate
the socket works just fine,
as the attached program demonstrates (also shows lazy reverse
exponential startup), and I
see nothing in the original reuseport patches that indicate that this
is a bad idea.

I'd do this if it was simple (because it gives the EPOLLET behavior I
am looking for), but
it requires some new fields in SocketPort to handle the CLOEXEC
switcharoos, as we will
have two of the same socket open at the same time, one spawned before
the connection came in
to give to the child, and another spawned right after the connection
came in, to hold onto for the
next connection. I'd really like to avoid that since SO_REUSEPORT does
not need it.
>
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> _______________________________________________
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel

ELF>	@@ @8@@@@@@ÀÀ@@@@ÔÔ ØØ`Ø`ÀÈ ðð`ð`àà@@DDPåtdÌÌ@Ì@44Qåtd/lib64/ld-linux-x86-64.so.2GNU GNUXÂØdú÷0íBa!^

	P6=De[ /Kyk*libc.so.6setuidsockethtonsepoll_waitforklistengetpidprintfmemsetbindsetsockoptepoll_ctlcloseepoll_create1acceptsleep__libc_start_mainwrite__gmon_start__GLIBC_2.9GLIBC_2.3.2GLIBC_2.2.5ii
ri	·ui	ÃÐ`
ð`ø````` `(`0`	8`
@`H`P`
X```h`p`x``HìH
 HÀtè³HÄÃÿ5
 ÿ%
 @ÿ%
 héàÿÿÿÿ%
 héÐÿÿÿÿ%
 héÀÿÿÿÿ%
 hé°ÿÿÿÿ%z
 hé ÿÿÿÿ%r
 héÿÿÿÿ%j
 héÿÿÿÿ%b
 hépÿÿÿÿ%Z
 hé`ÿÿÿÿ%R
 h	éPÿÿÿÿ%J
 h
é@ÿÿÿÿ%B
 hé0ÿÿÿÿ%:
 hé ÿÿÿÿ%2
 h
éÿÿÿÿ%*
 héÿÿÿÿ%"
 héðþÿÿÿ%
 héàþÿÿÿ%
 héÐþÿÿÿ%

 héÀþÿÿ1íIÑ^HâHäðPTIÇÀ@HÇÁð
@HÇÇp
@è'ÿÿÿôfD¸`UH-`HøHåw]Ã¸HÀtô]¿`ÿà¸`UH-`HÁøHåHÂHÁê?HÐHÑøu]ÃºHÒtô]HÆ¿`ÿâ=q	 uUHåè~ÿÿÿ]Æ^	 óÃ@H=  t¸HÀtU¿è`HåÿÐ]é{ÿÿÿésÿÿÿUHåHì ÇEü}øHuðH4%@HuÐ<%@}ØèÌýÿÿH<% @Æ°èÛýÿÿ¾HºH½°þÿÿþÿÿèÊýÿÿÇEà³fÇ°þÿÿÇ´þÿÿEàfÁ·ùèýÿÿ¿¾ºf²þÿÿèWþÿÿEì=H<%@°èZýÿÿEüé¾ºA¸H¤þÿÿÇ¤þÿÿ}ìHÁèæüÿÿ=H<%Â@°èýÿÿEüéPºH°þÿÿ}ìHÆèmýÿÿ=H<%Ú@°èÓüÿÿEüé¾}ìèýÿÿ=H<%í@°è¤üÿÿEüéè¿èRýÿÿEè=H<%@°èuüÿÿEüé¹¾HMÐ}èUìèüÿÿ=H<%@°è?üÿÿEüééHuÀº¹ÿÿÿÿ}èèüÿÿ=H<%6@°èüÿÿEüéFèÅüÿÿ¨þÿÿ½¨þÿÿ&èºûÿÿH<%O@Æ°èÉûÿÿþÿÿ¿èÇþÿÿèOüÿÿ þÿÿ½ þÿÿH<%_@°èûÿÿþÿÿHþÿÿHþÿÿ}ìHÆèûûÿÿEä=8è8ûÿÿH<%w@Æ°èGûÿÿ¿<|þÿÿèçûÿÿÇEüxþÿÿéqH4%@Hº}äèÛúÿÿHpþÿÿèßúÿÿH<%@Æ°èîúÿÿ}älþÿÿèûÿÿ=H<%¶@°èÆúÿÿEüé
éöþÿÿéþÿÿEüHÄ ]ÃHl$ØLd$àH-ß L%Ð H\$ÐLl$èLt$ðL|$øHì8L)åAÿIöHÁýIÕ1ÛèáùÿÿHít@LêLöDÿAÿÜHÃH9ëuêH\$Hl$Ld$Ll$ Lt$(L|$0HÄ8ÃóÃHìHÄÃmain PID %d
socket() failed: %m
setsockopt() failed: %mbind() failed: %m
listen() failed: %m
epoll_create1() failed: %m
epoll_ctl() failed: %m
epoll_wait() failed: %m
PID %d started
setuid(1000) failed: %mPID %d accept() failed: %m
foo
PID %d accept()ed connection
close() failed: %m
;0tøÿÿ|´ùÿÿL¤úÿÿ¤$þÿÿÄ´þÿÿìzRx`ùÿÿ*zRx$ð÷ÿÿ@FJw?;*3$"DøùÿÿxAC
$dXýÿÿJf@XÀýÿÿ@
@ 
@@
@Ø`à``@õþÿo@@ @
ÏØ`ÈP@8@	þÿÿoø@ÿÿÿoðÿÿoÐ@ð`V@f@v@@@¦@¶@Æ@Ö@æ@ö@	@	@&	@6	@F	@V	@f	@v	@GCC: (Debian 4.8.2-4) 4.8.2Debian clang version 3.4-3 (tags/RELEASE_34/rc2) (based on LLVM 3.4)GCC: (Debian 4.7.3-9) 4.7.3.symtab.strtab.shstrtab.interp.note.ABI-tag.note.gnu.build-id.gnu.hash.dynsym.dynstr.gnu.version.gnu.version_r.rela.dyn.rela.plt.init.text.fini.rodata.eh_frame_hdr.eh_frame.init_array.fini_array.jcr.dynamic.got.got.plt.data.bss.comment@#@ 1<@<$H`@`Döÿÿo@N @ àV@Ï^ÿÿÿoÐ@Ð(kþÿÿoø@ø@z8@8P@PÈ
@@@@@	@	@	 @:¨Ì@Ì4¶@ÔÀØ`ØÌà`àØè`èÝð`ðàæÐ`ÐëØ`Ø°ô`ú`ÿ0}à°.	$@@<@`@@ @@Ð@	ø@
8@P@@
@@	@@@Ì@@Ø`à`è`ð`Ð`Ø```ñÿè`°	@.à	@A 
@W`fà`@
@Ø`¸ñÿñÿÅÐ@Óè`ñÿßà`ðð`ùØ`Ø`"@2 N `Yq`@¥¸Ìàó
)`6 E`R@að
@q `	@`p
@x¢ºÌ àô` .AZ@`rcrtstuff.c__JCR_LIST__deregister_tm_clonesregister_tm_clones__do_global_dtors_auxcompleted.6392__do_global_dtors_aux_fini_array_entryframe_dummy__frame_dummy_init_array_entryepoll_test.c__FRAME_END____JCR_END____init_array_end_DYNAMIC__init_array_start_GLOBAL_OFFSET_TABLE___libc_csu_fini_ITM_deregisterTMCloneTabledata_startsetsockopt@@GLIBC_2.2.5write@@GLIBC_2.2.5getpid@@GLIBC_2.2.5_edata_finihtons@@GLIBC_2.2.5printf@@GLIBC_2.2.5memset@@GLIBC_2.2.5close@@GLIBC_2.2.5epoll_ctl@@GLIBC_2.3.2__libc_start_main@@GLIBC_2.2.5__data_start__gmon_start____dso_handle_IO_stdin_used__libc_csu_initlisten@@GLIBC_2.2.5_end_start__bss_startmainepoll_wait@@GLIBC_2.3.2bind@@GLIBC_2.2.5_Jv_RegisterClassesaccept@@GLIBC_2.2.5__TMC_END___ITM_registerTMCloneTablesetuid@@GLIBC_2.2.5sleep@@GLIBC_2.2.5epoll_create1@@GLIBC_2.9_initfork@@GLIBC_2.2.5socket@@GLIBC_2.2.5

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: support Distribute=n to distribute to n workers

Reply via email to