Re: [nfsv4] AD Evaluation for draft-ietf-nfsv4-flex-files-14.txt

[Thomas Haynes <[e-mail]>]

Hi Spencer,

Thanks for the review - I had to think and organize a response on what you were asking.

Tom

On Nov 7, 2017, at 11:42 AM, Spencer Dawkins at IETF <[e-mail]<mailto:[e-mail]>> wrote:

Hi, authors,

I apologize for not jumping on this draft when publication was requested - it shows up with that status in the datatracker, but a quick search of my e-mail archive doesn't show the corresponding notification e-mail to me. But, having noticed that I should be reading it ...

I have some questions, which I'd like to work through before requesting Last Call. Most are harmless, and a few are requirements language questions, but I really need to understand the IANA Considerations section note about Table 3.

Just administratively, I know that automatic I-D submissions are closed until next Monday, and NFSv4 isn't meeting at IETF 100, so I'd expect to wait until after IETF 100 to request Last Call (since we typically extend Last Calls that overlap meeting weeks anyway). If the chairs would like for me to handle this in another way, please let me know.

As an author, I am fine with that constraint.


And thanks for your work on this.

Spencer (D)

--

I think

   This document does
   not provide a standard's track control protocol.

should be "standard track control protocol”.

Ack


I think

   The requirements for the a
   control protocol are specified in [RFC5661] and clarified in
   [pNFSLayouts].

should either be "for a control protocol" or "for the control protocol", but not both.

Ack



I don't understand what "need not be a protocol as normally understood" in

     control protocol:  is the particular mechanism that an implementation
      of a layout type would use to meet the control communication
      requirement for that layout type.  This need not be a protocol as
      normally understood.  In some cases the same protocol may be used
      as a control protocol and storage protocol.

means. You do expand the definition in the next sentence, but only explaining something about a protocol as I understand protocols. Could you also add an example of the kind of thing you're thinking of, as "not a protocol”?

I thought it did :-)

How about:

   This need not be a protocol dedicated to the control communication. For
   example, the control protocol could be defined over a storage protocol.

If this still isn’t clear (which I admit may be the case), then we will have to point to a specific example.



Is it possible to add a sentence or two in Section 2, that explains the tradeoffs between loose and tight coupling? I understand there are two alternatives (and the differences are described throughout the doc), but don't understand at a high level how I'd decide which one to implement, if I was starting from scratch.

How about:

While tight coupling can provide a better integration
of the metadata server to the storage devices, it
requires either a standard track control protocol
or that a vendor implements both the metadata
server and the storage devices. In turn, the loose
coupling allows the flexibility of utilizing any storage
device which support the storage protocol.



I'm not a genius of "code in drafts", but shouldn't

  /// /*
   ///  * Copyright (c) 2012 IETF Trust and the persons identified
   ///  * as authors of the code. All rights reserved.
   ///  *

be 2017? The template in [LEGAL] is "insert current year" …

Lol - is it current year of the current draft or current year as of when we started?



I'm seeing major and minor version numbers, but I'm not seeing mention of https://datatracker.ietf.org/doc/rfc8178/ -style extensions. Does that matter? Or would support for extensions simply be detected in the way described at the end of https://tools.ietf.org/html/rfc8178#section-4.3?


Ahh, I was going to suggest another paragraph when I realized that this document
is built on top of the pNFS Layout Types Registry of RFC5661 (see Section 16)
and as such does not present any extensions along the lines of RFC8178.

I.e., we allow for a NFS4.1 implementation of the flex files layout type due to RFC5661,
while RFC8178 would disallow extensions to NFSv4.1.



I'm fine with all of the SHOULDs and SHOULD NOTs in

  The client is free to use any of the network addresses as a
   destination to send storage device requests.  If some network
   addresses are less desirable paths to the data than others, then the
   MDS SHOULD NOT include those network addresses in ffda_netaddrs.  If
   less desirable network addresses exist to provide failover, the
   RECOMMENDED method to offer the addresses is to provide them in a
   replacement device-ID-to-device-address mapping, or a replacement
   device ID.  When a client finds no response from the storage device
   using all addresses available in ffda_netaddrs, it SHOULD send a
   GETDEVICEINFO to attempt to replace the existing device-ID-to-device-
   address mappings.  If the MDS detects that all network paths
   represented by ffda_netaddrs are unavailable, the MDS SHOULD send a
   CB_NOTIFY_DEVICEID (if the client has indicated it wants device ID
   notifications for changed device IDs) to change the device-ID-to-
   device-address mappings to the available addresses.  If the device ID
   itself will be replaced, the MDS SHOULD recall all layouts with the
   device ID, and thus force the client to get new layouts and device ID
   mappings via LAYOUTGET and GETDEVICEINFO.

except the last one. Can you help me understand why the MDS would not recall all layouts and force clients to get new layouts and mappings?


Argh, should not have MDS in the first place!

Should be metadata server.

The metadata server does not need to recall the layouts as the client should
check any layout stateids it has with the device ID which is being replaced.

https://tools.ietf.org/html/rfc5661#section-18.40.4

   o  CB_NOTIFY_DEVICEID deletes a device ID.  If the client believes it
      has layouts that refer to the device ID, then it is possible that
      layouts referring to the deleted device ID have been revoked.  The
      client should send a TEST_STATEID request using the stateid for
      each layout that might have been revoked.

Although in contradiction: https://tools.ietf.org/html/rfc5661#section-20.12.3

   NOTIFY4_DEVICEID_DELETE
      Deletes a device ID from the mappings.  This notification MUST NOT
      be sent if the client has a layout that refers to the device ID.
      In other words, if the server is sending a delete device ID
      notification, one of the following is true for layouts associated
      with the layout type:

      *  The client never had a layout referring to that device ID.

      *  The client has returned all layouts referring to that device
         ID.

      *  The server has revoked all layouts referring to that device ID.

      The notification is encoded in a value of data type
      notify_deviceid_delete4.  After a server deletes a device ID, it
      MUST NOT reuse that device ID for the same layout type until the
      client ID is deleted.

Which implies that the SHOULD is a MUST, except that the client
can deal with it via TEST_STATEID.

Anyone want to chime in?


Is it correct that there's no difference between the client's action when receiving

  NFS4ERR_LAYOUTTRYLATER:  there is some issue preventing the layout
      from being granted.  If the client already has an appropriate
      layout, it should continue with I/O to the storage devices.

and

   NFS4ERR_DELAY:  there is some issue preventing the layout from being
      granted.  If the client already has an appropriate layout, it
      should not continue with I/O to the storage devices.

? That wouldn't surprise me, but I thought I should ask.

Yes, there is a difference. Look at what is recommended if the client already has an appropriate layout.



I think

  In [RFC5661], the file layout type is defined such that the
   relationship between multipathing and filehandles can result in
   either 0, 1, or N filehandles (see Section 13.3).  Some rationals for
   this are clustered servers which share the same filehandle or
   allowing for multiple read-only copies of the file on the same
   storage device.

should be "rationales”.

Ack


Could you help me understand why

  The metadata server SHOULD recall any outstanding layouts to allow it
   exclusive write access to the stripes being recovered and to prevent
   other clients from hitting the same error condition.  In these cases,
   the server MUST complete recovery before handing out any new layouts
   to the affected byte ranges.

is not a MUST? ("Why wouldn't the MDS do that?”)

Ack.

Although what might have been meant here is that :

  The metadata server MUST recall any outstanding layout, on those segments
  which are being recovered, to allow it …

I.e., the SHOULD allowed non-impacted layout segments to not be recalled.


Is "hot" a term of art for the NFS community? (If so, is there a reference with a definition that could be included on first use?)

Not sure I would say it is a term of art for NFS, but perhaps one for computing in general.

And unfortunately, a search on "hot computer performance” or "hot spot computer performance”, oh
wait, that is it:

https://en.wikipedia.org/wiki/Hot_spot_(computer_programming)

But not so sure I am going to reference WikiPedia here...


I'm really confused by

  Note, [RFC5661] should have also defined (see Table 3):

   +-------------------------------+------+-----------+-----+----------+
   | Recallable Object Type Name   | Valu | RFC       | How | Minor    |
   |                               | e    |           |     | Versions |
   +-------------------------------+------+-----------+-----+----------+
   | RCA4_TYPE_MASK_OTHER_LAYOUT_M | 12   | [RFC5661] | L   | 1        |
   | IN                            |      |           |     |          |
   | RCA4_TYPE_MASK_OTHER_LAYOUT_M | 15   | [RFC5661] | L   | 1        |
   | AX                            |      |           |     |          |
   +-------------------------------+------+-----------+-----+----------+

              Table 3: Recallable Object Type Assignments

As best I can tell, https://www.iana.org/assignments/nfsv4-recallable-object-types/nfsv4-recallable-object-types.xhtml#nfsv4-recallable-object-types-1 defines them as

RCA4_TYPE_MASK_OBJ_LAYOUT_MIN 8 [RFC5661] L 1
RCA4_TYPE_MASK_OBJ_LAYOUT_MAX 9 [RFC5661] L 1

so, the values don't match. What am I missing? Is this a request to IANA to change the assigned values, or something else?

No, you have the wrong values. Note in https://tools.ietf.org/html/rfc5661#section-20.6.1 :


   const RCA4_TYPE_MASK_RDATA_DLG          = 0;
   const RCA4_TYPE_MASK_WDATA_DLG          = 1;
   const RCA4_TYPE_MASK_DIR_DLG            = 2;
   const RCA4_TYPE_MASK_FILE_LAYOUT        = 3;
   const RCA4_TYPE_MASK_BLK_LAYOUT         = 4;
   const RCA4_TYPE_MASK_OBJ_LAYOUT_MIN     = 8;
   const RCA4_TYPE_MASK_OBJ_LAYOUT_MAX     = 9;
   const RCA4_TYPE_MASK_OTHER_LAYOUT_MIN   = 12;
   const RCA4_TYPE_MASK_OTHER_LAYOUT_MAX   = 15;


This section simply points out that RFC5661 should have defined these values in
https://tools.ietf.org/html/rfc5661#section-22.3.1

They might be covered as reserved for private use in
https://www.iana.org/assignments/nfsv4-recallable-object-types/nfsv4-recallable-object-types.xhtml#nfsv4-recallable-object-types-1.
But that should really be called out.

Although perhaps this (in https://tools.ietf.org/html/rfc5661#section-20.6.3) covers that:

   RCA4_TYPE_MASK_OTHER_LAYOUT_MIN to RCA4_TYPE_MASK_OTHER_LAYOUT_MAX

      This range is reserved for telling the client to recall layouts of
      experimental or site-specific layout types (see Section 3.3.13).

I found it all confusing and I’m willing to drop it if you believe it is covered in RFC5661.