All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jeff Hostetler" <git@jeffhostetler.com>,
	"Jeff King" <peff@peff.net>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Chris Torek" <chris.torek@gmail.com>,
	"Jeff Hostetler" <jeffhost@microsoft.com>
Subject: Re: [PATCH v5 11/12] simple-ipc: add Unix domain socket implementation
Date: Tue, 09 Mar 2021 16:08:21 -0800	[thread overview]
Message-ID: <xmqq7dmfzx8q.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <f2e3b046cc8f8ad5662f65262810c7414cc1569d.1615302157.git.gitgitgadget@gmail.com> (Jeff Hostetler via GitGitGadget's message of "Tue, 09 Mar 2021 15:02:36 +0000")

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +/*
> + * This value was chosen at random.
> + */
> +#define WAIT_STEP_MS (50)

... and never used.  Is this supposed to be used as the hardcoded
value 50 below ...

> +
> +/*
> + * Try to connect to the server.  If the server is just starting up or
> + * is very busy, we may not get a connection the first time.
> + */
> +static enum ipc_active_state connect_to_server(
> +	const char *path,
> +	int timeout_ms,
> +	const struct ipc_client_connect_options *options,
> +	int *pfd)
> +{
> +	int wait_ms = 50;

... here?

> +	int k;
> +
> +	*pfd = -1;
> +
> +	for (k = 0; k < timeout_ms; k += wait_ms) {
> +		int fd = unix_stream_connect(path, options->uds_disallow_chdir);
> +
> +		if (fd != -1) {
> +			*pfd = fd;
> +			return IPC_STATE__LISTENING;
> +		}
> +
> +		if (errno == ENOENT) {
> +			if (!options->wait_if_not_found)
> +				return IPC_STATE__PATH_NOT_FOUND;
> +
> +			goto sleep_and_try_again;
> +		}
> + ...
> +		return IPC_STATE__OTHER_ERROR;
> +
> +	sleep_and_try_again:
> +		sleep_millisec(wait_ms);

Or, since there is nothing like exponential back-off implemented
here which may want to modify wait_ms variable, perhaps use the
constant directly here and where k is incremented?

> +/*
> + * A randomly chosen timeout value.
> + */
> +#define MY_CONNECTION_TIMEOUT_MS (1000)

Even if it may have been "randomly chosen", there should be some
criteria to judge if the value is sensible, right?  IOW, I have a
suspicion that I would regret if I randomly chose 5 (or 3600000)
instead of 1000.  How would we figure that 1000 acceptable but not
5?

Perhaps explain that criterion here, e.g. "... value that ought to
be long enough to establish connection locally as long as the box is
not loaded unusably heavily" or something?


  reply	other threads:[~2021-03-10  0:09 UTC|newest]

Thread overview: 178+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-12 15:31 [PATCH 00/10] [RFC] Simple IPC Mechanism Jeff Hostetler via GitGitGadget
2021-01-12 15:31 ` [PATCH 01/10] pkt-line: use stack rather than static buffer in packet_write_gently() Jeff Hostetler via GitGitGadget
2021-01-13 13:29   ` Jeff King
2021-01-25 19:34     ` Jeff Hostetler
2021-01-12 15:31 ` [PATCH 02/10] pkt-line: (optionally) libify the packet readers Johannes Schindelin via GitGitGadget
2021-01-12 15:31 ` [PATCH 03/10] pkt-line: optionally skip the flush packet in write_packetized_from_buf() Johannes Schindelin via GitGitGadget
2021-01-12 15:31 ` [PATCH 04/10] pkt-line: accept additional options in read_packetized_to_strbuf() Johannes Schindelin via GitGitGadget
2021-01-12 15:31 ` [PATCH 05/10] simple-ipc: design documentation for new IPC mechanism Jeff Hostetler via GitGitGadget
2021-01-12 16:40   ` Ævar Arnfjörð Bjarmason
2021-01-12 15:31 ` [PATCH 06/10] simple-ipc: add win32 implementation Jeff Hostetler via GitGitGadget
2021-01-12 15:31 ` [PATCH 07/10] unix-socket: create gentle version of unix_stream_listen() Jeff Hostetler via GitGitGadget
2021-01-13 14:06   ` Jeff King
2021-01-14  1:19     ` Chris Torek
2021-01-12 15:31 ` [PATCH 08/10] unix-socket: add no-chdir option to unix_stream_listen_gently() Jeff Hostetler via GitGitGadget
2021-01-12 15:31 ` [PATCH 09/10] simple-ipc: add t/helper/test-simple-ipc and t0052 Jeff Hostetler via GitGitGadget
2021-01-12 15:31 ` [PATCH 10/10] simple-ipc: add Unix domain socket implementation Jeff Hostetler via GitGitGadget
2021-01-12 16:50 ` [PATCH 00/10] [RFC] Simple IPC Mechanism Ævar Arnfjörð Bjarmason
2021-01-12 18:25   ` Jeff Hostetler
2021-01-12 20:01 ` Junio C Hamano
2021-01-12 23:25   ` Jeff Hostetler
2021-01-13  0:13     ` Junio C Hamano
2021-01-13  0:32       ` Jeff Hostetler
2021-01-13 13:46     ` Jeff King
2021-01-13 15:48       ` Ævar Arnfjörð Bjarmason
2021-02-01 19:45 ` [PATCH v2 00/14] " Jeff Hostetler via GitGitGadget
2021-02-01 19:45   ` [PATCH v2 01/14] ci/install-depends: attempt to fix "brew cask" stuff Junio C Hamano via GitGitGadget
2021-02-01 19:45   ` [PATCH v2 02/14] pkt-line: promote static buffer in packet_write_gently() to callers Jeff Hostetler via GitGitGadget
2021-02-02  9:41     ` Jeff King
2021-02-02 20:33       ` Jeff Hostetler
2021-02-02 22:54       ` Johannes Schindelin
2021-02-03  4:52         ` Jeff King
2021-02-01 19:45   ` [PATCH v2 03/14] pkt-line: add write_packetized_from_buf2() that takes scratch buffer Jeff Hostetler via GitGitGadget
2021-02-02  9:44     ` Jeff King
2021-02-01 19:45   ` [PATCH v2 04/14] pkt-line: optionally skip the flush packet in write_packetized_from_buf() Johannes Schindelin via GitGitGadget
2021-02-02  9:48     ` Jeff King
2021-02-02 22:56       ` Johannes Schindelin
2021-02-05 18:30       ` Jeff Hostetler
2021-02-01 19:45   ` [PATCH v2 05/14] pkt-line: (optionally) libify the packet readers Johannes Schindelin via GitGitGadget
2021-02-01 19:45   ` [PATCH v2 06/14] pkt-line: accept additional options in read_packetized_to_strbuf() Johannes Schindelin via GitGitGadget
2021-02-11  1:52     ` Taylor Blau
2021-02-01 19:45   ` [PATCH v2 07/14] simple-ipc: design documentation for new IPC mechanism Jeff Hostetler via GitGitGadget
2021-02-01 19:45   ` [PATCH v2 08/14] simple-ipc: add win32 implementation Jeff Hostetler via GitGitGadget
2021-02-01 19:45   ` [PATCH v2 09/14] simple-ipc: add t/helper/test-simple-ipc and t0052 Jeff Hostetler via GitGitGadget
2021-02-02 21:35     ` SZEDER Gábor
2021-02-03  4:36       ` Jeff King
2021-02-09 15:45       ` Jeff Hostetler
2021-02-05 19:38     ` SZEDER Gábor
2021-02-01 19:45   ` [PATCH v2 10/14] unix-socket: elimiate static unix_stream_socket() helper function Jeff Hostetler via GitGitGadget
2021-02-02  9:54     ` Jeff King
2021-02-02  9:58     ` Jeff King
2021-02-01 19:45   ` [PATCH v2 11/14] unix-socket: add options to unix_stream_listen() Jeff Hostetler via GitGitGadget
2021-02-02 10:14     ` Jeff King
2021-02-05 23:28       ` Jeff Hostetler
2021-02-09 16:32         ` Jeff King
2021-02-09 17:39           ` Jeff Hostetler
2021-02-10 15:55             ` Jeff King
2021-02-10 21:31               ` Jeff Hostetler
2021-02-01 19:45   ` [PATCH v2 12/14] unix-socket: add no-chdir option " Jeff Hostetler via GitGitGadget
2021-02-02 10:26     ` Jeff King
2021-02-01 19:45   ` [PATCH v2 13/14] unix-socket: do not call die in unix_stream_connect() Jeff Hostetler via GitGitGadget
2021-02-01 19:45   ` [PATCH v2 14/14] simple-ipc: add Unix domain socket implementation Jeff Hostetler via GitGitGadget
2021-02-01 22:20   ` [PATCH v2 00/14] Simple IPC Mechanism Junio C Hamano
2021-02-01 23:26     ` Jeff Hostetler
2021-02-02 23:07       ` Johannes Schindelin
2021-02-04 19:08         ` Junio C Hamano
2021-02-05 13:19           ` candidate branches for `maint`, was " Johannes Schindelin
2021-02-05 19:55             ` Junio C Hamano
2021-02-13  0:09   ` [PATCH v3 00/12] " Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently() Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 02/12] pkt-line: do not issue flush packets in write_packetized_*() Johannes Schindelin via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 03/12] pkt-line: (optionally) libify the packet readers Johannes Schindelin via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 04/12] pkt-line: add options argument to read_packetized_to_strbuf() Johannes Schindelin via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 05/12] simple-ipc: design documentation for new IPC mechanism Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 06/12] simple-ipc: add win32 implementation Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 07/12] unix-socket: elimiate static unix_stream_socket() helper function Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 08/12] unix-socket: add backlog size option to unix_stream_listen() Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 09/12] unix-socket: disallow chdir() when creating unix domain sockets Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 10/12] unix-socket: create `unix_stream_server__listen_with_lock()` Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 11/12] simple-ipc: add Unix domain socket implementation Jeff Hostetler via GitGitGadget
2021-02-13  0:09     ` [PATCH v3 12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool Jeff Hostetler via GitGitGadget
2021-02-13  9:30       ` SZEDER Gábor
2021-02-16 15:53         ` Jeff Hostetler
2021-02-17 21:48     ` [PATCH v4 00/12] Simple IPC Mechanism Jeff Hostetler via GitGitGadget
2021-02-17 21:48       ` [PATCH v4 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently() Jeff Hostetler via GitGitGadget
2021-02-26  7:21         ` Jeff King
2021-02-26 19:52           ` Jeff Hostetler
2021-02-26 20:43             ` Jeff King
2021-03-03 19:38             ` Junio C Hamano
2021-03-04 13:29               ` Jeff Hostetler
2021-03-04 20:26                 ` Junio C Hamano
2021-02-17 21:48       ` [PATCH v4 02/12] pkt-line: do not issue flush packets in write_packetized_*() Johannes Schindelin via GitGitGadget
2021-02-17 21:48       ` [PATCH v4 03/12] pkt-line: (optionally) libify the packet readers Johannes Schindelin via GitGitGadget
2021-03-03 19:53         ` Junio C Hamano
2021-03-04 14:17           ` Jeff Hostetler
2021-03-04 14:40             ` Jeff King
2021-03-04 20:28               ` Junio C Hamano
2021-02-17 21:48       ` [PATCH v4 04/12] pkt-line: add options argument to read_packetized_to_strbuf() Johannes Schindelin via GitGitGadget
2021-02-17 21:48       ` [PATCH v4 05/12] simple-ipc: design documentation for new IPC mechanism Jeff Hostetler via GitGitGadget
2021-03-03 20:19         ` Junio C Hamano
2021-02-17 21:48       ` [PATCH v4 06/12] simple-ipc: add win32 implementation Jeff Hostetler via GitGitGadget
2021-02-17 21:48       ` [PATCH v4 07/12] unix-socket: elimiate static unix_stream_socket() helper function Jeff Hostetler via GitGitGadget
2021-02-26  7:25         ` Jeff King
2021-03-03 20:41         ` Junio C Hamano
2021-02-17 21:48       ` [PATCH v4 08/12] unix-socket: add backlog size option to unix_stream_listen() Jeff Hostetler via GitGitGadget
2021-02-26  7:30         ` Jeff King
2021-03-03 20:54           ` Junio C Hamano
2021-02-17 21:48       ` [PATCH v4 09/12] unix-socket: disallow chdir() when creating unix domain sockets Jeff Hostetler via GitGitGadget
2021-03-03 22:53         ` Junio C Hamano
2021-03-04 14:56           ` Jeff King
2021-03-04 20:34             ` Junio C Hamano
2021-03-04 23:34               ` Junio C Hamano
2021-03-05  9:02                 ` Jeff King
2021-03-05  9:25                   ` Jeff King
2021-03-05 11:59                     ` Chris Torek
2021-03-05 17:33                       ` Jeff Hostetler
2021-03-05 17:53                         ` Junio C Hamano
2021-03-05 21:30               ` Jeff Hostetler
2021-03-05 21:52                 ` Junio C Hamano
2021-02-17 21:48       ` [PATCH v4 10/12] unix-socket: create `unix_stream_server__listen_with_lock()` Jeff Hostetler via GitGitGadget
2021-02-26  7:56         ` Jeff King
2021-03-02 23:50           ` Jeff Hostetler
2021-03-04 15:13             ` Jeff King
2021-02-17 21:48       ` [PATCH v4 11/12] simple-ipc: add Unix domain socket implementation Jeff Hostetler via GitGitGadget
2021-02-17 21:48       ` [PATCH v4 12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool Jeff Hostetler via GitGitGadget
2021-03-02  9:44         ` Jeff King
2021-03-03 15:25           ` Jeff Hostetler
2021-02-25 19:39       ` [PATCH v4 00/12] Simple IPC Mechanism Junio C Hamano
2021-02-26  7:59         ` Jeff King
2021-02-26 20:18           ` Jeff Hostetler
2021-02-26 20:50             ` Jeff King
2021-03-03 19:29               ` Junio C Hamano
2021-03-09 15:02       ` [PATCH v5 " Jeff Hostetler via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently() Jeff Hostetler via GitGitGadget
2021-03-09 23:48           ` Junio C Hamano
2021-03-11 19:29             ` Jeff King
2021-03-11 20:32               ` Junio C Hamano
2021-03-11 20:53                 ` Jeff King
2021-03-09 15:02         ` [PATCH v5 02/12] pkt-line: do not issue flush packets in write_packetized_*() Johannes Schindelin via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 03/12] pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option Johannes Schindelin via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 04/12] pkt-line: add options argument to read_packetized_to_strbuf() Johannes Schindelin via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 05/12] simple-ipc: design documentation for new IPC mechanism Jeff Hostetler via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 06/12] simple-ipc: add win32 implementation Jeff Hostetler via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 07/12] unix-socket: eliminate static unix_stream_socket() helper function Jeff Hostetler via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 08/12] unix-socket: add backlog size option to unix_stream_listen() Jeff Hostetler via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 09/12] unix-socket: disallow chdir() when creating unix domain sockets Jeff Hostetler via GitGitGadget
2021-03-09 15:02         ` [PATCH v5 10/12] unix-stream-server: create unix domain socket under lock Jeff Hostetler via GitGitGadget
2021-03-10  0:18           ` Junio C Hamano
2021-03-09 15:02         ` [PATCH v5 11/12] simple-ipc: add Unix domain socket implementation Jeff Hostetler via GitGitGadget
2021-03-10  0:08           ` Junio C Hamano [this message]
2021-03-15 19:56             ` Jeff Hostetler
2021-03-09 15:02         ` [PATCH v5 12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool Jeff Hostetler via GitGitGadget
2021-03-09 23:28         ` [PATCH v5 00/12] Simple IPC Mechanism Junio C Hamano
2021-03-15 21:08         ` [PATCH v6 " Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently() Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 02/12] pkt-line: do not issue flush packets in write_packetized_*() Johannes Schindelin via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 03/12] pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option Johannes Schindelin via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 04/12] pkt-line: add options argument to read_packetized_to_strbuf() Johannes Schindelin via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 05/12] simple-ipc: design documentation for new IPC mechanism Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 06/12] simple-ipc: add win32 implementation Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 07/12] unix-socket: eliminate static unix_stream_socket() helper function Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 08/12] unix-socket: add backlog size option to unix_stream_listen() Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 09/12] unix-socket: disallow chdir() when creating unix domain sockets Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 10/12] unix-stream-server: create unix domain socket under lock Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 11/12] simple-ipc: add Unix domain socket implementation Jeff Hostetler via GitGitGadget
2021-03-15 21:08           ` [PATCH v6 12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool Jeff Hostetler via GitGitGadget
2021-03-22 10:29           ` [PATCH v7 00/12] Simple IPC Mechanism Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently() Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 02/12] pkt-line: do not issue flush packets in write_packetized_*() Johannes Schindelin via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 03/12] pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option Johannes Schindelin via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 04/12] pkt-line: add options argument to read_packetized_to_strbuf() Johannes Schindelin via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 05/12] simple-ipc: design documentation for new IPC mechanism Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 06/12] simple-ipc: add win32 implementation Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 07/12] unix-socket: eliminate static unix_stream_socket() helper function Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 08/12] unix-socket: add backlog size option to unix_stream_listen() Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 09/12] unix-socket: disallow chdir() when creating unix domain sockets Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 10/12] unix-stream-server: create unix domain socket under lock Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 11/12] simple-ipc: add Unix domain socket implementation Jeff Hostetler via GitGitGadget
2021-03-22 10:29             ` [PATCH v7 12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool Jeff Hostetler via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7dmfzx8q.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=chris.torek@gmail.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jeffhost@microsoft.com \
    --cc=peff@peff.net \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.