public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@redhat.com>
To: Calum Mackay <calum.mackay@oracle.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [RFC] pynfs: add courteous server tests
Date: Tue, 16 Feb 2021 17:47:14 -0500	[thread overview]
Message-ID: <YCxLcm8FFFWzZELr@pick.fieldses.org> (raw)
In-Reply-To: <47d31c15-7467-6abb-9e62-96ffca1c6ec0@oracle.com>

On Tue, Feb 16, 2021 at 10:04:05PM +0000, Calum Mackay wrote:
> hi Bruce,
> 
> At Chuck's suggestion, I've added an initial PyNFS test to aid work on a
> courteous server. A simple test, along the lines you indicated, that locks a
> file, waits twice the lease period, and tries to unlock:
> 
> OK -> PASS (courteous server)
> BADSESSION -> WARNING (discourteous server)
> 
> 
> Before sending my patch, Chuck asked me to add the second test you
> suggested:
> 
> 	- A second test creates a new client, acquires a file lock, and
> 	  waits two lease periods.  Then creates a second client, which
> 	  attempts to acquire the lock.  The second client should
> 	  succeed.
> 
> 
> 
> This doesn't seem to differentiate between these three cases:
> 
> 1. a discourteous server, which invalidates the client 1 state, and frees
> all client 1's locks, upon lease expiry, then allows client 2 to lock the
> file. The above test spec would result in a PASS for a discourteous server,
> which doesn't seem right.

Apologies for the confusing suggestion.  I think all I really wanted to
verify was that the server will grant the lock to a second client after
a lease period has gone by.

That's a simple test that *any* server (courteous or discourteous)
should pass.  (We probably do have a test for that at least in the 4.0
case.  It'd be nice to have one in the 4.1 case.  Maybe just look into
porting some 4.0 tests over to 4.1.)

Anyway, it seems like a simple thing that would be useful to verify
while doing courteous server implementation.  I mean, we want to make
sure we don't accidentally implement a "courteous" server that works by
just never dropping any client's state ever.

> 2. a broad-grained courteous server, which invalidates the client 1 state,
> and frees all client 1's locks, because of conflicting access from client 2
> (after client 1's lease expiry), who is then granted the lock. A PASS here
> would be correct.
> 
> 3. a fine-grained courteous server, which persists the session, but revokes
> that particular client 1 lock, because of conflicting access from client 2
> (after client 1's lease expiry), who is granted the lock. A PASS here would
> be correct.

> If I've read it right, the test could differentiate between cases 2) and 3),
> by having client 1 try to unlock, after client 2 successfully locks, where
> client 1 will see either BADSESSION (case 2) or SOME_STATE_REVOKED / EXPIRED
> (case 3).

Sounds like a good idea to test those two cases.

> But we don't need to differentiate cases 2) and 3), since a PASS
> would be correct in either case.

But, yes, they're both "courteous" cases.

I do wish sometimes that we had states other than "PASS/FAIL/WARN";
sometimes you want to know stuff about a server that isn't just whether
it's "right" or not.

Bit it's not really a big deal.  Note if you leave the "all" flag off a
test, it won't be run by default.  And if a test flagged "courteous" but
not "all" fails on a correct but uncourteous server, that's probably
fine.

I think it'd also be fine to WARN about anything short of the
finest-grained possible behavior.  You can give more details in the
wording of the warning.

> However that won't differentiate between cases 1) and 2), where client 1
> will see BADSESSION in each case. Yet case 1) ought to result in a WARNING,
> and case 2) in a PASS?

We could also do something like:

	client 1 acquires two different locks.
	wait a lease period or two
	client 2 attempts to acquire one of the locks.
	client 1 attempts to unlock the other one.

And that'd be another way to test whether we have a coarse- or fine-
grained courteous server.

My test suggestions were very off-the-cuff, if you have ideas then go
for it.

--b.


      reply	other threads:[~2021-02-16 22:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-16 22:04 [RFC] pynfs: add courteous server tests Calum Mackay
2021-02-16 22:47 ` J. Bruce Fields [this message]

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=YCxLcm8FFFWzZELr@pick.fieldses.org \
    --to=bfields@redhat.com \
    --cc=calum.mackay@oracle.com \
    --cc=linux-nfs@vger.kernel.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