Re: [PATCH v2] http: add http.maxReceiveSpeed to limit receiving speed of "git-receive-pack"

From: Jiang Xin <worldhello.net@gmail.com>
To: "chenan.xxw" <haoyurenzhuxia@gmail.com>
Cc: Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	chenan.xxw@alibaba-inc.com, Jeff King <peff@peff.net>
Subject: Re: [PATCH v2] http: add http.maxReceiveSpeed to limit receiving speed of "git-receive-pack"
Date: Tue, 24 Aug 2021 08:31:42 +0800	[thread overview]
Message-ID: <CANYiYbEL2T897jMZFg_F-AgomrEj3EcEC2ZTyBZMB550k_PPbQ@mail.gmail.com> (raw)
In-Reply-To: <20210823160455.32397-1-chenan.xxw@alibaba-inc.com>

On Tue, Aug 24, 2021 at 12:05 AM chenan.xxw <haoyurenzhuxia@gmail.com> wrote:
>
> In order to avoid hogging all the available network bandwidth, users may want to
> limit the speed of receiving traffic for "git clone" or "git fetch".
>
> Add `http.maxReceiveSpeed` to limit receiving speed of `git-receive-pack`.
> Can be overridden by `GIT_HTTP_MAX_RECEIVE_SPEED` environment variable.
>
> The default is unlimited, same if the value is 0. The default unit is Bytes/s,
> common unit suffixes of k, m, or g are supported.
>
> This configuration is valid for `clone`, `fetch`, `pull` commands of the https
> protocol.
>
> Signed-off-by: chenan.xxw <chenan.xxw@alibaba-inc.com>

Your email address in the s-o-b is different from the "From" header.
That will generate a commit with a different author email. This is
because you use gmail as the SMTP, but the value of "user.email"
config variable is not a valid alias email of Gmail. A workaround is
use a different "user.email" config for git-send-email, and
git-send-email will add a "From: your <real@email>" as the first line
of the body. E.g.:

    git -c user.email="haoyurenzhuxia@gmail.com" send-email <patch-file>

BTW, use your real username (Xia Xiaowen) instead of the nickname "chenan.xxw".

    git config --global user.name "Xia Xiaowen"
    git commit --reset-author --amend

> ---
>  Documentation/config/http.txt |  4 ++++
>  http.c                        | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/Documentation/config/http.txt b/Documentation/config/http.txt
> index 7003661c0d..9b9fb5e9c7 100644
> --- a/Documentation/config/http.txt
> +++ b/Documentation/config/http.txt
> @@ -235,6 +235,10 @@ http.lowSpeedLimit, http.lowSpeedTime::
>         Can be overridden by the `GIT_HTTP_LOW_SPEED_LIMIT` and
>         `GIT_HTTP_LOW_SPEED_TIME` environment variables.
>
> +http.maxReceiveSpeed::
> +       Limit the speed of receiving traffic, defaults to unlimited. Can be
> +       overridden by the `GIT_HTTP_MAX_RECEIVE_SPEED` environment variable.
> +
>  http.noEPSV::
>         A boolean which disables using of EPSV ftp command by curl.
>         This can helpful with some "poor" ftp servers which don't
> diff --git a/http.c b/http.c
> index 8119247149..b12d192ffe 100644
> --- a/http.c
> +++ b/http.c
> @@ -83,6 +83,7 @@ static const char *ssl_pinnedkey;
>  static const char *ssl_cainfo;
>  static long curl_low_speed_limit = -1;
>  static long curl_low_speed_time = -1;
> +static long curl_max_receive_speed;

You can set default value -1 for curl_max_receive_speed, just like
curl_low_speed_limit does.
I wonder if you can rename the variable name to curl_max_speed_limit
for both upload and download.

>  static int curl_ftp_no_epsv;
>  static const char *curl_http_proxy;
>  static const char *http_proxy_authmethod;
> @@ -362,6 +363,13 @@ static int http_options(const char *var, const char *value, void *cb)
>                 return 0;
>         }
>
> +       if (!strcmp("http.maxreceivespeed", var)) {

Can we use a better name, such as "http.maxspeedlimit" ?  And make
some changes to limit both upload and download over HTTP protocol.

> +               curl_max_receive_speed = (long)git_config_int(var, value);
> +               if (curl_max_receive_speed < 0)
> +                       die(_("negative values are not allowed for http.maxreceivespeed"));
> +               return 0;
> +       }
> +
>         if (!strcmp("http.noepsv", var)) {
>                 curl_ftp_no_epsv = git_config_bool(var, value);
>                 return 0;
> @@ -974,6 +982,10 @@ static CURL *get_curl_handle(void)
>                                  curl_low_speed_time);
>         }
>
> +       if (curl_max_receive_speed >= 0)
> +               curl_easy_setopt(result, CURLOPT_MAX_RECV_SPEED_LARGE,
> +                                (curl_off_t)curl_max_receive_speed);
> +

You can also set "CURLOPT_MAX_SEND_SPEED_LARGE" to limit bandwidth for git-push.

>         curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
>  #if LIBCURL_VERSION_NUM >= 0x071301
>         curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
> @@ -1105,6 +1117,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
>  {
>         char *low_speed_limit;
>         char *low_speed_time;
> +       char *mrs;
> +       static const char mrs_env[] = "GIT_HTTP_MAX_RECEIVE_SPEED";
>         char *normalized_url;
>         struct urlmatch_config config = { STRING_LIST_INIT_DUP };
>
> @@ -1197,6 +1211,13 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
>         if (low_speed_time != NULL)
>                 curl_low_speed_time = strtol(low_speed_time, NULL, 10);
>
> +       mrs = getenv(mrs_env);
> +       if (mrs != NULL) {
> +               curl_max_receive_speed = strtol(mrs, NULL, 10);
> +               if (curl_max_receive_speed < 0)
> +                       die(_("negative values are not allowed for %s"), mrs_env);
> +       }

You introduced a new l10n message for translation. Why not make it as
simple as parsing env "GIT_HTTP_LOW_SPEED_LIMIT" or
"GIT_HTTP_LOW_SPEED_TIME" above?

--
Jiang Xin