public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
From: Mike Christie <mchristi@redhat.com>
To: Josef Bacik <josef@toxicpanda.com>, Hou Pu <houpu.main@gmail.com>,
	axboe@kernel.dk
Cc: linux-block@vger.kernel.org, nbd@other.debian.org,
	Hou Pu <houpu@bytedance.com>
Subject: Re: [PATCH 2/2] nbd: requeue command if the soecket is changed
Date: Tue, 3 Mar 2020 16:06:55 -0600	[thread overview]
Message-ID: <5E5ED4FF.8020209@redhat.com> (raw)
In-Reply-To: <34249aaa-7f0e-d0f4-7c1a-28aee9bddaa0@toxicpanda.com>

On 03/03/2020 03:13 PM, Josef Bacik wrote:
> On 2/28/20 1:40 AM, Hou Pu wrote:
>> In commit 2da22da5734 (nbd: fix zero cmd timeout handling v2),
>> it is allowed to reset timer when it fires if tag_set.timeout
>> is set to zero. If the server is shutdown and a new socket
>> is reconfigured, the request should be requeued to be processed by
>> new server instead of waiting for response from the old one.
>>
>> Signed-off-by: Hou Pu <houpu@bytedance.com>
> 
> I'm confused by this, if we get here we've already timed out and
> requeued once right?  Why do we need to requeue again?  Thanks,
> 

We may not have timed out already. If the tag_set.timeout=0, then the
block timer will fire every 30 seconds. This could be the first time the
timer has fired. If it has fired multiple times already then it still
would not have been requeued because the num_connections=1 code just
does a BLK_EH_RESET_TIMER when timeout=0 and does not have support for
detecting reconnects.

In this second patch if timeout=0 and num_connections=1 we restart the
command when the command timer fires and we detect a new connection
(nsock->cookie has incremented).

I was saying in the last patch, maybe waiting for reconnect is wrong.
Does a cmd timeout=0 mean to wait for a reconnect or in this patch
should we do:

1. if timeout=0, num_connections=1, and the cmd timer fires and the
conneciton is marked dead then requeue the command.
2. we then rely on the dead_conn_timeout code to decide how long to wait
for a reconnect.


  reply	other threads:[~2020-03-03 22:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28  6:40 [PATCH v2 0/2] nbd: requeue request if only one connection is configured Hou Pu
2020-02-28  6:40 ` [PATCH 1/2] nbd: enable replace socket " Hou Pu
2020-03-03 21:12   ` Josef Bacik
2020-03-03 21:48     ` Mike Christie
2020-03-04  5:41       ` Hou Pu
2020-03-04 18:48   ` Josef Bacik
2020-02-28  6:40 ` [PATCH 2/2] nbd: requeue command if the soecket is changed Hou Pu
2020-03-03 21:13   ` Josef Bacik
2020-03-03 22:06     ` Mike Christie [this message]
2020-03-03 22:21       ` Mike Christie
2020-03-04  7:13         ` Hou Pu
2020-03-04 18:48   ` Josef Bacik
2020-03-12 13:59 ` [PATCH v2 0/2] nbd: requeue request if only one connection is configured Jens Axboe
2020-03-13 11:29   ` Hou Pu
  -- strict thread matches above, loose matches on Subject: below --
2020-02-19  6:31 [PATCH 0/2] " Hou Pu
2020-02-19  6:31 ` [PATCH 2/2] nbd: requeue command if the soecket is changed Hou Pu

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=5E5ED4FF.8020209@redhat.com \
    --to=mchristi@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=houpu.main@gmail.com \
    --cc=houpu@bytedance.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=nbd@other.debian.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox