2020-08-11 17:28:04

by Olga Kornievskaia

[permalink] [raw]
Subject: Questions about GETATTRs

Hi Trond,

I'd like to discuss a couple of commits that are dealing with GETATTRs.

First is the recent fix in: commit
3a39e778690500066b31fe982d18e2e394d3bce2 "nfs: set invalid blocks
after NFSv4 writes". The fact that CLOSE's GETATTR doesn't query for
available space leads to stale attribute data but marking attributes
invalid instead leads to an additional standalone GETATTR if
attributes are after writing to the file. Question: why not change
CLOSE's GETATTR to query for additional attributes instead?

Another commit I would like to inquire about is: commit
3ecefc9295991eaaad4c67915c6384e5d18cc632 "NFSv4: Don't request
close-to-open attribute when holding a delegation". After this commit
if the client application queries for attributes after writing it
triggers a gettattr (because we didn't get them in the close
compound). I understand that it was an optimization but it leads to an
extra RPC in some workloads so how can we claim that one saving is
better than another?

Thank you for the feedback.


2020-08-12 18:47:19

by Trond Myklebust

[permalink] [raw]
Subject: Re: Questions about GETATTRs

Hi Olga,

On Tue, 2020-08-11 at 13:27 -0400, Olga Kornievskaia wrote:
> Hi Trond,
>
> I'd like to discuss a couple of commits that are dealing with
> GETATTRs.
>
> First is the recent fix in: commit
> 3a39e778690500066b31fe982d18e2e394d3bce2 "nfs: set invalid blocks
> after NFSv4 writes". The fact that CLOSE's GETATTR doesn't query for
> available space leads to stale attribute data but marking attributes
> invalid instead leads to an additional standalone GETATTR if
> attributes are after writing to the file. Question: why not change
> CLOSE's GETATTR to query for additional attributes instead?
>

So, the point of the CLOSE getattr is to ensure that we manage cache
consistency. While we could ask for further attributes, there are
servers out there for which this might cause slowdowns (in particular
some servers that operate in a clustered mode and that have a sublist
of attributes that they prefetch). I'm therefore open to discussions
around this, but I want to make sure that we give folks a chance to
test against their favourite servers first.

> Another commit I would like to inquire about is: commit
> 3ecefc9295991eaaad4c67915c6384e5d18cc632 "NFSv4: Don't request
> close-to-open attribute when holding a delegation". After this commit
> if the client application queries for attributes after writing it
> triggers a gettattr (because we didn't get them in the close
> compound). I understand that it was an optimization but it leads to
> an
> extra RPC in some workloads so how can we claim that one saving is
> better than another?
>
> Thank you for the feedback.

When we hold a delegation, the call to CLOSE is usually not considered
a data/metadata synchronisation point. That means we're not guaranteed
to synchronously flush writes (see nfs4_delegation_flush_on_close() in
nfs4_file_flush()), and we usually don't return pNFS layouts.

When we're not flushing writes, then the obvious consequence is that
the GETATTR might just end up getting discarded by the client, while
holding onto the layout can cause the GETATTR to be inefficient on some
servers. Those are the real reasons for the optimisation.

Cheers
Trond

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]