[RFC] pynfs: add courteous server tests

[RFC] pynfs: add courteous server tests

[Calum Mackay]

[-- Attachment #1.1: Type: text/plain, Size: 2013 bytes --]

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.

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.

Or am I misreading your suggestion?


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). But we don't need to differentiate cases 2) and 3), 
since a PASS would be correct in either case.

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?


cheers,
calum.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [RFC] pynfs: add courteous server tests

[J. Bruce Fields]

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.