threads / patch / 46016

v5send-email: --batch-size to work around some SMTP server limit

Subject: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

## tl;dr

15 messages between May 21, 2017 and Feb 12, 2018. Diffs are folded; open one to read it.

replies: 14people: 6as markdown or json

xiaoqiang zhao· May 21, 2017, 12:59 UTC · lore

Some email servers (e.g. smtp.163.com) limit the number emails to be sent per session(connection) and this will lead to a faliure when sending many messages.

Teach send-email to disconnect after sending a number of messages (configurable via the --batch-size=<num> option), wait for a few seconds (configurable via the --relogin-delay=<seconds> option) and reconnect, to work around such a limit.

Also add this two configuration option.
Note:
   Re-authentication will happen every $<batch-size> messages, so it
will be much more acceptable if you use some form of credential helper
(e.g. the 'sendemail.smtppass' config option), otherwise you will have
to retype password every time when asked.
Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 Documentation/config.txt               |  8 ++++++++
 Documentation/git-send-email.txt       | 11 +++++++++++
 contrib/completion/git-completion.bash |  2 ++
 git-send-email.perl                    | 18 ++++++++++++++++++
 4 files changed, 39 insertions(+)
Show changes to 4 files +39 −0

Documentation/config.txt, Documentation/git-send-email.txt, contrib/completion/git-completion.bash, git-send-email.perl

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 96e9cf8b7..173ed63f6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2917,6 +2917,14 @@ sendemail.xmailer::
 sendemail.signedoffcc (deprecated)::
 	Deprecated alias for `sendemail.signedoffbycc`.
 
+sendemail.smtpbatchsize::
+	Number of messages to be sent per connection, after that a relogin
+	will happen. if the value is 0 or undefined, send all messages in
+	one connection.
+
+sendemail.smtprelogindelay::
+	Seconds wait before reconnecting to smtp server.
+
 showbranch.default::
 	The default set of branches for linkgit:git-show-branch[1].
 	See linkgit:git-show-branch[1].
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 9d66166f6..5380d8c95 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -248,6 +248,17 @@ must be used for each option.
 	commands and replies will be printed. Useful to debug TLS
 	connection and authentication problems.
 
+--batch-size=<num>::
+	Some email servers (e.g. smtp.163.com) limit the number emails to be
+	sent per session(connection) and this will lead to a faliure when
+	sending many messages.  With this option, send-email will disconnect after
+	sending $<num> messages and wait for a few seconds (see --relogin-delay)
+	and reconnect, to work around such a limit.
+
+--relogin-delay=<int>::
+	Waiting $<int> seconds before reconnecting to smtp server. Used together
+	with --batch-size option.
+
 Automating
 ~~~~~~~~~~
 
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1ed0a09fe..933e7badf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2611,6 +2611,8 @@ _git_config ()
 		sendemail.thread
 		sendemail.to
 		sendemail.validate
+		sendemail.smtpbatchsize
+		sendemail.smtprelogindelay
 		showbranch.default
 		status.relativePaths
 		status.showUntrackedFiles
diff --git a/git-send-email.perl b/git-send-email.perl
index eea0a517f..8a1ee0f0d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -81,6 +81,10 @@ git send-email --dump-aliases
                                      This setting forces to use one of the listed mechanisms.
     --smtp-debug            <0|1>  * Disable, enable Net::SMTP debug.
 
+    --batch-size            <int>  * send max <int> message per connection.
+    --relogin-delay         <int>  * delay <int> seconds between two successive login.
+                                     This option can only be used with --batch-size
+
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
@@ -153,6 +157,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
 my $have_mail_address = eval { require Mail::Address; 1 };
 my $smtp;
 my $auth;
+my $num_sent = 0;
 
 # Regexes for RFC 2047 productions.
 my $re_token = qr/[^][()<>@,;:\\"\/?.= \000-\037\177-\377]+/;
@@ -216,6 +221,7 @@ my ($cover_cc, $cover_to);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
+my ($batch_size, $relogin_delay);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -247,6 +253,8 @@ my %config_settings = (
     "smtppass" => \$smtp_authpass,
     "smtpdomain" => \$smtp_domain,
     "smtpauth" => \$smtp_auth,
+    "smtpbatchsize" => \$batch_size,
+    "smtprelogindelay" => \$relogin_delay,
     "to" => \@initial_to,
     "tocmd" => \$to_cmd,
     "cc" => \@initial_cc,
@@ -358,6 +366,8 @@ $rc = GetOptions(
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
 		    "no-xmailer" => sub {$use_xmailer = 0},
+		    "batch-size=i" => \$batch_size,
+		    "relogin-delay=i" => \$relogin_delay,
 	 );
 
 usage() if $help;
@@ -1664,6 +1674,14 @@ foreach my $t (@files) {
 		}
 	}
 	$message_id = undef;
+	$num_sent++;
+	if (defined $batch_size && $num_sent == $batch_size) {
+		$num_sent = 0;
+		$smtp->quit if defined $smtp;
+		undef $smtp;
+		undef $auth;
+		sleep($relogin_delay) if defined $relogin_delay;
+	}
 }
 
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
-- 
2.13.0.rc2.116.g565bdd0fc.dirty
Junio C Hamano· May 22, 2017, 02:14 UTC · re: xiaoqiang zhao · lore

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

I think this is almost perfect.
I'd propose squashing the patch below to
 - Add cross reference between config and option
 - Spell configuration variables in camelCase to mimic other
   sendemail.* variables
 - Spell SMTP in all caps to mimic other parts of the manual
 - Suggest use of credential helper in the document to help the end
   users, not in the proposed log message.

If you are fine with all of these changes, there is no need to resend (you can say so and I can locally squash these in). Of course, anyone is very welcome to point out documentation bugs I may be introducing with this patch.

Thanks.
-- >8 --
Subject: fixup! send-email: --batch-size to work around some SMTP server limit

Some email servers (e.g. smtp.163.com) limit the number emails to be sent per session (connection) and this will lead to a faliure when sending many messages.

Teach send-email to disconnect after sending a number of messages (configurable via the --batch-size=<num> option), wait for a few seconds (configurable via the --relogin-delay=<seconds> option) and reconnect, to work around such a limit.

Also add two configuration variables to give these options the default.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt         |  8 +++++---
 Documentation/git-send-email.txt | 12 ++++++++----
 2 files changed, 13 insertions(+), 7 deletions(-)
Show changes to 2 files +13 −7

Documentation/config.txt, Documentation/git-send-email.txt

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a13315ed69..ee4a111878 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2914,13 +2914,15 @@ sendemail.xmailer::
 sendemail.signedoffcc (deprecated)::
 	Deprecated alias for `sendemail.signedoffbycc`.
 
-sendemail.smtpbatchsize::
+sendemail.smtpBatchSize::
 	Number of messages to be sent per connection, after that a relogin
-	will happen. if the value is 0 or undefined, send all messages in
+	will happen.  If the value is 0 or undefined, send all messages in
 	one connection.
+	See also the `--batch-size` option of linkgit:git-send-email[1].
 
-sendemail.smtprelogindelay::
+sendemail.smtpReloginDelay::
 	Seconds wait before reconnecting to smtp server.
+	See also the `--relogin-delay` option of linkgit:git-send-email[1].
 
 showbranch.default::
 	The default set of branches for linkgit:git-show-branch[1].
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 5380d8c956..79b418bfa5 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -250,14 +250,18 @@ must be used for each option.
 
 --batch-size=<num>::
 	Some email servers (e.g. smtp.163.com) limit the number emails to be
-	sent per session(connection) and this will lead to a faliure when
+	sent per session (connection) and this will lead to a faliure when
 	sending many messages.  With this option, send-email will disconnect after
 	sending $<num> messages and wait for a few seconds (see --relogin-delay)
-	and reconnect, to work around such a limit.
+	and reconnect, to work around such a limit.  You may want to
+	use some form of credential helper to avoid having to retype
+	your password every time this happens.  Defaults to the
+	`sendemail.smtpBatchSize` configuration variable.
 
 --relogin-delay=<int>::
-	Waiting $<int> seconds before reconnecting to smtp server. Used together
-	with --batch-size option.
+	Waiting $<int> seconds before reconnecting to SMTP server. Used together
+	with --batch-size option.  Defaults to the `sendemail.smtpReloginDelay`
+	configuration variable.
 
 Automating
 ~~~~~~~~~~
-- 
2.13.0-440-g3ce6d2d5b8
Zhaoxiangqiang· May 22, 2017, 03:19 UTC · re: Junio C Hamano · lore

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

Junio C Hamano 于 2017 年 5 月 22 日 星期一 写道:
Show 41 quoted lines
> I think this is almost perfect.
> 
> I'd propose squashing the patch below to
> 
>  - Add cross reference between config and option
> 
>  - Spell configuration variables in camelCase to mimic other
>    sendemail.* variables
> 
>  - Spell SMTP in all caps to mimic other parts of the manual
> 
>  - Suggest use of credential helper in the document to help the end
>    users, not in the proposed log message.
> 
> If you are fine with all of these changes, there is no need to
> resend (you can say so and I can locally squash these in).  Of
> course, anyone is very welcome to point out documentation bugs I may
> be introducing with this patch.
> 
> Thanks.
> 
> -- >8 --
> Subject: fixup! send-email: --batch-size to work around some SMTP server limit
> 
> Some email servers (e.g. smtp.163.com) limit the number emails to be
> sent per session (connection) and this will lead to a faliure when
> sending many messages.
> 
> Teach send-email to disconnect after sending a number of messages
> (configurable via the --batch-size=<num> option), wait for a few
> seconds (configurable via the --relogin-delay=<seconds> option) and
> reconnect, to work around such a limit.
> 
> Also add two configuration variables to give these options the default.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/config.txt         |  8 +++++---
>  Documentation/git-send-email.txt | 12 ++++++++----
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
It's fine with me, just  go ahead.
Thank you  Hamano!
 
Show 54 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index a13315ed69..ee4a111878 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2914,13 +2914,15 @@ sendemail.xmailer::
>  sendemail.signedoffcc (deprecated)::
>  	Deprecated alias for `sendemail.signedoffbycc`.
>  
> -sendemail.smtpbatchsize::
> +sendemail.smtpBatchSize::
>  	Number of messages to be sent per connection, after that a relogin
> -	will happen. if the value is 0 or undefined, send all messages in
> +	will happen.  If the value is 0 or undefined, send all messages in
>  	one connection.
> +	See also the `--batch-size` option of linkgit:git-send-email[1].
>  
> -sendemail.smtprelogindelay::
> +sendemail.smtpReloginDelay::
>  	Seconds wait before reconnecting to smtp server.
> +	See also the `--relogin-delay` option of linkgit:git-send-email[1].
>  
>  showbranch.default::
>  	The default set of branches for linkgit:git-show-branch[1].
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 5380d8c956..79b418bfa5 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -250,14 +250,18 @@ must be used for each option.
>  
>  --batch-size=<num>::
>  	Some email servers (e.g. smtp.163.com) limit the number emails to be
> -	sent per session(connection) and this will lead to a faliure when
> +	sent per session (connection) and this will lead to a faliure when
>  	sending many messages.  With this option, send-email will disconnect after
>  	sending $<num> messages and wait for a few seconds (see --relogin-delay)
> -	and reconnect, to work around such a limit.
> +	and reconnect, to work around such a limit.  You may want to
> +	use some form of credential helper to avoid having to retype
> +	your password every time this happens.  Defaults to the
> +	`sendemail.smtpBatchSize` configuration variable.
>  
>  --relogin-delay=<int>::
> -	Waiting $<int> seconds before reconnecting to smtp server. Used together
> -	with --batch-size option.
> +	Waiting $<int> seconds before reconnecting to SMTP server. Used together
> +	with --batch-size option.  Defaults to the `sendemail.smtpReloginDelay`
> +	configuration variable.
>  
>  Automating
>  ~~~~~~~~~~
> -- 
> 2.13.0-440-g3ce6d2d5b8
> 
>
Stefan Beller· Feb 7, 2018, 19:45 UTC · re: Junio C Hamano · lore

[PATCH] send-email: have default batch size when relogin delay is given

When the batch size is neither configured nor given on the command line, but the relogin delay is given, then the user is not using the the feature as intended. But as the user gave a relogin delay, there is clearly the intention to delay sending out emails. Assume a batch size of 1 instead of silently ignoring the given relogin delay.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-send-email.perl | 6 ++++++
 1 file changed, 6 insertions(+)
Show changes to git-send-email.perl +6 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..5672e05b98 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,12 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
 	if $format_patch and not $repo;
 
+if (defined $relogin_delay) {
+	if (not defined $batch_size) {
+		$batch_size = 1;
+	}
+}
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.15.1.433.g936d1b9894.dirty
Eric Sunshine· Feb 7, 2018, 19:50 UTC · re: Stefan Beller · lore

Re: [PATCH] send-email: have default batch size when relogin delay is given

On Wed, Feb 7, 2018 at 2:45 PM, Stefan Beller <sbeller@google.com> wrote:
Show 15 quoted lines
> When the batch size is neither configured nor given on the command
> line, but the relogin delay is given, then the user is not using the
> the feature as intended. But as the user gave a relogin delay, there is
> clearly the intention to delay sending out emails. Assume a batch size
> of 1 instead of silently ignoring the given relogin delay.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -379,6 +379,12 @@ unless ($rc) {
> +if (defined $relogin_delay) {
> +       if (not defined $batch_size) {
> +               $batch_size = 1;
> +       }
> +}

Maybe also print a message that this batch size has been used as default lest the user wonder why it's sending "slowly" without apparently batching anything.

Alternately, complain and die if both options are not specified.
Stefan Beller· Feb 7, 2018, 23:43 UTC · re: Eric Sunshine · lore

[PATCH] send-email: error out when relogin delay is missing

When the batch size is neither configured nor given on the command line, but the relogin delay is given, then the current code ignores the relogin delay setting.

This is unsafe as there was some intention when setting the batch size. One workaround would be to just assume a batch size of 1 as a default. This however may be bad UX, as then the user may wonder why it is sending slowly without apparent batching.

Error out for now instead of potentially confusing the user. As 5453b83bdf (send-email: --batch-size to work around some SMTP server limit, 2017-05-21) lays out, we rather want to not have this interface anyway and would rather want to react on the server throttling dynamically.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-send-email.perl | 3 +++
 1 file changed, 3 insertions(+)
Show changes to git-send-email.perl +3 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..bc0d3ade16 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,9 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
 	if $format_patch and not $repo;
 
+die __("When a batch size is given, the relogin delay must be set\n")
+	if defined $relogin_delay and not defined $batch_size;
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.16.0.rc1.238.g530d649a79-goog
xiaoqiang zhao· Feb 8, 2018, 05:45 UTC · re: Stefan Beller · lore

Re: [PATCH] send-email: error out when relogin delay is missing

Show 5 quoted lines
> 在 2018年2月8日,上午7:43,Stefan Beller <sbeller@google.com> 写道:
> 
> +die __("When a batch size is given, the relogin delay must be set\n")
> +    if defined $relogin_delay and not defined $batch_size;
> +
 
According the code, maybe you want to say “When relogin delay is given, a batch size must be set “ ?
Eric Sunshine· Feb 8, 2018, 08:08 UTC · re: Stefan Beller · lore

Re: [PATCH] send-email: error out when relogin delay is missing

On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller <sbeller@google.com> wrote:
Show 13 quoted lines
> [...]
> Error out for now instead of potentially confusing the user.
> As 5453b83bdf (send-email: --batch-size to work around some SMTP
> server limit, 2017-05-21) lays out, we rather want to not have this
> interface anyway and would rather want to react on the server throttling
> dynamically.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -379,6 +379,9 @@ unless ($rc) {
> +die __("When a batch size is given, the relogin delay must be set\n")
> +       if defined $relogin_delay and not defined $batch_size;

This only makes sense is 'batch-size' is specified but not 'relogin'. If the other way around, then the error is confusing. How about this instead?

    "--batch-size and --relogin must be specified together"
...or something.
Stefan Beller· Feb 8, 2018, 18:21 UTC · re: Eric Sunshine · lore

Re: [PATCH] send-email: error out when relogin delay is missing

On Thu, Feb 8, 2018 at 12:08 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 22 quoted lines
> On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller <sbeller@google.com> wrote:
>> [...]
>> Error out for now instead of potentially confusing the user.
>> As 5453b83bdf (send-email: --batch-size to work around some SMTP
>> server limit, 2017-05-21) lays out, we rather want to not have this
>> interface anyway and would rather want to react on the server throttling
>> dynamically.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> @@ -379,6 +379,9 @@ unless ($rc) {
>> +die __("When a batch size is given, the relogin delay must be set\n")
>> +       if defined $relogin_delay and not defined $batch_size;
>
> This only makes sense is 'batch-size' is specified but not 'relogin'.
> If the other way around, then the error is confusing. How about this
> instead?
>
>     "--batch-size and --relogin must be specified together"
>
> ...or something.

I like this for its expressiveness as it would have helped me a lot. I dislike this because it is incorrect when you use the config options instead of command line arguments.

Stefan
Eric Sunshine· Feb 8, 2018, 22:00 UTC · re: Stefan Beller · lore

Re: [PATCH] send-email: error out when relogin delay is missing

On Thu, Feb 8, 2018 at 1:21 PM, Stefan Beller <sbeller@google.com> wrote:
Show 14 quoted lines
> On Thu, Feb 8, 2018 at 12:08 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller <sbeller@google.com> wrote:
>>> +die __("When a batch size is given, the relogin delay must be set\n")
>>> +       if defined $relogin_delay and not defined $batch_size;
>>
>> This only makes sense is 'batch-size' is specified but not 'relogin'.
>> If the other way around, then the error is confusing. How about this
>> instead?
>>     "--batch-size and --relogin must be specified together"
>> ...or something.
>
> I like this for its expressiveness as it would have helped me a lot.
> I dislike this because it is incorrect when you use the config options
> instead of command line arguments.
Perhaps:
    "`batch-size` and `relogin` must be specified together
      (via command-line or configuration option)"
Stefan Beller· Feb 12, 2018, 19:44 UTC · re: Eric Sunshine · lore

[PATCH] send-email: error out when relogin delay is missing

When the batch size is neither configured nor given on the command line, but the relogin delay is given, then the current code ignores the relogin delay setting.

This is unsafe as there was some intention when setting the batch size. One workaround would be to just assume a batch size of 1 as a default. This however may be bad UX, as then the user may wonder why it is sending slowly without apparent batching.

Error out for now instead of potentially confusing the user. As 5453b83bdf (send-email: --batch-size to work around some SMTP server limit, 2017-05-21) lays out, we rather want to not have this interface anyway and would rather want to react on the server throttling dynamically.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-send-email.perl | 4 ++++
 1 file changed, 4 insertions(+)
Show changes to git-send-email.perl +4 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..f7913f7c2c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,10 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
 	if $format_patch and not $repo;
 
+die __("`batch-size` and `relogin` must be specified together " .
+	"(via command-line or configuration option)\n")
+	if defined $relogin_delay and not defined $batch_size;
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.15.1.433.g936d1b9894.dirty
Ævar Arnfjörð Bjarmason· May 22, 2017, 09:26 UTC · re: xiaoqiang zhao · lore

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

On Sun, May 21, 2017 at 2:59 PM, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
> Some email servers (e.g. smtp.163.com) limit the number emails to be
> sent per session(connection) and this will lead to a faliure when
> sending many messages.
This OK to me, the nits I had are addressed by Junio's reply.

Looking at this the Nth time now though I wonder about this approach in general. In all your E-Mails I don't think you ever said /what/ sort of error you had from the SMTP server, you just said you had a failure or an error, I assume you hit one of the die's in the send_message() function. Can you paste the actual error you get without this patch?

I wonder if something like this would Just Work for this case without any configuration or command-line options, with the added benefit of just working for anyone with transitory SMTP issues as well (patch posted with -w, full version at https://github.com/avar/git/commit/acb60c4bde50bdcb62b71ed46f49617e2caef84e.patch):

Show changes to git-send-email.perl +20 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 8a1ee0f0d4..c2d85236d1 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1363,6 +1363,10 @@ EOF
                        die __("The required SMTP server is not
properly defined.")
                }

+               my $num_tries = 0;
+               my $max_tries = 5;
+       smtp_again:
+               eval {
                        if ($smtp_encryption eq 'ssl') {
                                $smtp_server_port ||= 465; # ssmtp
                                require Net::SMTP::SSL;
@@ -1429,6 +1433,22 @@ EOF
                        }
                        $smtp->dataend() or die $smtp->message;
                        $smtp->code =~ /250|200/ or die
sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
+                       1;
+               } or do {
+                       my $error = $@ || "Zombie Error";
+
+                       warn sprintf(__("Failed to send %s due to
error: %s"), $subject, $error);
+                       if ($num_tries++ < $max_tries) {
+                               $smtp->quit if defined $smtp;
+                               $smtp = undef;
+                               $auth = undef;
+                               my $sleep = $num_tries * 3; # 3, 6, 9, ...
+                               warn sprintf(__("This is retry %d/%d.
Sleeping %d before trying again"),
+                                            $num_tries, $max_tries, $sleep);
+                               sleep($sleep);
+                               goto smtp_again;
+                       }
+               };
        }
        if ($quiet) {
                printf($dry_run ? __("Dry-Sent %s\n") : __("Sent
%s\n"), $subject);

Now that's very much a WIP and I don't have a server like that to test against.

Having worked with SMTP a lot in a past life/job, I'd say it's *very*
likely that you're just getting a /^4/ error code from 163.com,
probably 421, which would make this logic even simpler. I.e. we could
just adjust this to back-off for /^4/ instead of trying to handle
arbitrary errors.

Anyway, I'm not interested in pursuing that WIP patch, and I don't
think perfect should be the enemy of the good here. Your patch works
for you, doesn't really damage anything else, so if you're not
interested in hacking up something like the above I think we should
just take it.

But I do think it would be very good to get a reply to you / details
in the commit message about what error you get exactly in this
scenario, see if you get better details with --smtp-debug, and if so
paste that (sans any secret info like user/password you don't want to
share).

Then if we're poking at this code in the future we can maybe just fix
this in some more general fashion while keeping this use-case in mind.
Junio C Hamano· May 23, 2017, 07:46 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 12 quoted lines
> Looking at this the Nth time now though I wonder about this approach
> in general. In all your E-Mails I don't think you ever said /what/
> sort of error you had from the SMTP server, you just said you had a
> failure or an error, I assume you hit one of the die's in the
> send_message() function. Can you paste the actual error you get
> without this patch?
>
> I wonder if something like this would Just Work for this case without
> any configuration or command-line options, with the added benefit of
> just working for anyone with transitory SMTP issues as well (patch
> posted with -w, full version at
> https://github.com/avar/git/commit/acb60c4bde50bdcb62b71ed46f49617e2caef84e.patch):

Yeah, if the issues users of 163.com are having can be resolved with a more general approach like this, that would be very much preferred.

Show 20 quoted lines
> Now that's very much a WIP and I don't have a server like that to test against.
>
> Having worked with SMTP a lot in a past life/job, I'd say it's *very*
> likely that you're just getting a /^4/ error code from 163.com,
> probably 421, which would make this logic even simpler. I.e. we could
> just adjust this to back-off for /^4/ instead of trying to handle
> arbitrary errors.
>
> Anyway, I'm not interested in pursuing that WIP patch, and I don't
> think perfect should be the enemy of the good here. Your patch works
> for you, doesn't really damage anything else, so if you're not
> interested in hacking up something like the above I think we should
> just take it.
>
>
> But I do think it would be very good to get a reply to you / details
> in the commit message about what error you get exactly in this
> scenario, see if you get better details with --smtp-debug, and if so
> paste that (sans any secret info like user/password you don't want to
> share).

Let's wait for a few days to see if xiaoqiang wants to take your outline of more general approach and polish it. I do prefer the "no config" solution as xiaoqiang won't be the only 163.com user, but Individual Contributors cannot be forced, so ...

Thanks.
Jan Viktorin· May 23, 2017, 08:30 UTC · re: Junio C Hamano · lore

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

On Tue, 23 May 2017 16:46:27 +0900 Junio C Hamano <gitster@pobox.com> wrote:

Show 8 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > Looking at this the Nth time now though I wonder about this approach
> > in general. In all your E-Mails I don't think you ever said /what/
> > sort of error you had from the SMTP server, you just said you had a
> > failure or an error, I assume you hit one of the die's in the
> > send_message() function. Can you paste the actual error you get
> > without this patch?
Hello,
I have issues with a company SMTP server that returns:

Net::SMTP::SSL=GLOB(0x20d6510)<<< 451 4.3.0 Please try again later, rate limited. 4.3.0 Please try again later, rate limited.

Unfortunately, I didn't find out the exact properties of the limit yet. It seems that sending more then 10 patches at once fails. Thus, I have to send longer patch sets in 2 rounds:

1. normal git send-email
2. git send-email --no-thread --in-reply-to="<COVER LETTER ID>" \
     <REST-OF-PATCHES>...
It is not exactly the same as sending all the patches at once.

The xiaoqiang's solution sounds promising to me. However, probably a more general solution would be to "just" enable sending a whole patch set in 2 rounds manually. But I didn't find any way how to do it right.

Regards Jan

Show 37 quoted lines
> >
> > I wonder if something like this would Just Work for this case without
> > any configuration or command-line options, with the added benefit of
> > just working for anyone with transitory SMTP issues as well (patch
> > posted with -w, full version at
> > https://github.com/avar/git/commit/acb60c4bde50bdcb62b71ed46f49617e2caef84e.patch):  
> 
> Yeah, if the issues users of 163.com are having can be resolved with
> a more general approach like this, that would be very much preferred.
> 
> > Now that's very much a WIP and I don't have a server like that to test against.
> >
> > Having worked with SMTP a lot in a past life/job, I'd say it's *very*
> > likely that you're just getting a /^4/ error code from 163.com,
> > probably 421, which would make this logic even simpler. I.e. we could
> > just adjust this to back-off for /^4/ instead of trying to handle
> > arbitrary errors.
> >
> > Anyway, I'm not interested in pursuing that WIP patch, and I don't
> > think perfect should be the enemy of the good here. Your patch works
> > for you, doesn't really damage anything else, so if you're not
> > interested in hacking up something like the above I think we should
> > just take it.
> >
> >
> > But I do think it would be very good to get a reply to you / details
> > in the commit message about what error you get exactly in this
> > scenario, see if you get better details with --smtp-debug, and if so
> > paste that (sans any secret info like user/password you don't want to
> > share).  
> 
> Let's wait for a few days to see if xiaoqiang wants to take your
> outline of more general approach and polish it.  I do prefer the "no
> config" solution as xiaoqiang won't be the only 163.com user, but
> Individual Contributors cannot be forced, so ...
> 
> Thanks.
赵小强· May 23, 2017, 12:55 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re:Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

At 2017-05-22 17:26:41, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com> wrote:
Show 14 quoted lines
>On Sun, May 21, 2017 at 2:59 PM, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
>> Some email servers (e.g. smtp.163.com) limit the number emails to be
>> sent per session(connection) and this will lead to a faliure when
>> sending many messages.
>
>This OK to me, the nits I had are addressed by Junio's reply.
>
>Looking at this the Nth time now though I wonder about this approach
>in general. In all your E-Mails I don't think you ever said /what/
>sort of error you had from the SMTP server, you just said you had a
>failure or an error, I assume you hit one of the die's in the
>send_message() function. Can you paste the actual error you get
>without this patch?
>
When I send a patch series which has 13 (plus cover) messages as a test, I got errors as follows and send-email quit when sending the 11th message:
MI:DMC 163 smtp14,EsCowAD3o71TKyRZTBlJHw--.20496S13 1495542613 http://mail.163.com/help/help_spam_16.htm?ip=1.203.183.150&hostid=smtp14&time=1495542613
Follow the link above, I find two error code:

•450 MI:DMC 当前连接发送的邮件数量超出限制。请减少每次连接中投递的邮件数量 •451 MI:DMC 当前连接发送的邮件数量超出限制。请控制每次连接中投递的邮件数量

Translate into English: •450 MI:DMC The number of messages sent execeeds the limits. Please reduce the number of messages to be sent per connection. •451 MI:DMC The number of messages sent execeeds the limits. Please control the number of messages to be sent per connection.

Although has different error code, but  says similar reason. Testing with --smtp-debug option produce the same error.
Show 72 quoted lines
>I wonder if something like this would Just Work for this case without
>any configuration or command-line options, with the added benefit of
>just working for anyone with transitory SMTP issues as well (patch
>posted with -w, full version at
>https://github.com/avar/git/commit/acb60c4bde50bdcb62b71ed46f49617e2caef84e.patch):
>
>diff --git a/git-send-email.perl b/git-send-email.perl
>index 8a1ee0f0d4..c2d85236d1 100755
>--- a/git-send-email.perl
>+++ b/git-send-email.perl
>@@ -1363,6 +1363,10 @@ EOF
>                        die __("The required SMTP server is not
>properly defined.")
>                }
>
>+               my $num_tries = 0;
>+               my $max_tries = 5;
>+       smtp_again:
>+               eval {
>                        if ($smtp_encryption eq 'ssl') {
>                                $smtp_server_port ||= 465; # ssmtp
>                                require Net::SMTP::SSL;
>@@ -1429,6 +1433,22 @@ EOF
>                        }
>                        $smtp->dataend() or die $smtp->message;
>                        $smtp->code =~ /250|200/ or die
>sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
>+                       1;
>+               } or do {
>+                       my $error = $@ || "Zombie Error";
>+
>+                       warn sprintf(__("Failed to send %s due to
>error: %s"), $subject, $error);
>+                       if ($num_tries++ < $max_tries) {
>+                               $smtp->quit if defined $smtp;
>+                               $smtp = undef;
>+                               $auth = undef;
>+                               my $sleep = $num_tries * 3; # 3, 6, 9, ...
>+                               warn sprintf(__("This is retry %d/%d.
>Sleeping %d before trying again"),
>+                                            $num_tries, $max_tries, $sleep);
>+                               sleep($sleep);
>+                               goto smtp_again;
>+                       }
>+               };
>        }
>        if ($quiet) {
>                printf($dry_run ? __("Dry-Sent %s\n") : __("Sent
>%s\n"), $subject);
>
>Now that's very much a WIP and I don't have a server like that to test against.
>
>Having worked with SMTP a lot in a past life/job, I'd say it's *very*
>likely that you're just getting a /^4/ error code from 163.com,
>probably 421, which would make this logic even simpler. I.e. we could
>just adjust this to back-off for /^4/ instead of trying to handle
>arbitrary errors.
>
>Anyway, I'm not interested in pursuing that WIP patch, and I don't
>think perfect should be the enemy of the good here. Your patch works
>for you, doesn't really damage anything else, so if you're not
>interested in hacking up something like the above I think we should
>just take it.
>
>But I do think it would be very good to get a reply to you / details
>in the commit message about what error you get exactly in this
>scenario, see if you get better details with --smtp-debug, and if so
>paste that (sans any secret info like user/password you don't want to
>share).
>
>Then if we're poking at this code in the future we can maybe just fix
>this in some more general fashion while keeping this use-case in mind.

← back to recent threads