Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-4215

Some expected improvements for OUT

Details

    • Improvement
    • Resolution: Unresolved
    • Minor
    • None
    • Lustre 2.6.0
    • 3
    • 11467

    Description

      1. OUT RPC service threads on MDT and OST using different reply portals confused the OUT RPC user.

      On MDT-side, it is:

                      .psc_buf                = {
                              .bc_nbufs               = MDS_NBUFS,
                              .bc_buf_size            = OUT_BUFSIZE,
                              .bc_req_max_size        = OUT_MAXREQSIZE,
                              .bc_rep_max_size        = OUT_MAXREPSIZE,
                              .bc_req_portal          = OUT_PORTAL,
                              .bc_rep_portal          = MDC_REPLY_PORTAL,
                      },
      

      On OST-side, it is:

                      .psc_buf                = {
                              .bc_nbufs               = OST_NBUFS,
                              .bc_buf_size            = OUT_BUFSIZE,
                              .bc_req_max_size        = OUT_MAXREQSIZE,
                              .bc_rep_max_size        = OUT_MAXREPSIZE,
                              .bc_req_portal          = OUT_PORTAL,
                              .bc_rep_portal          = OSC_REPLY_PORTAL,
                      },
      

      For the case that both MDT and OST runs on the same physical server node (especially for VM environment testing), when OSP wants to talk with OST via OUT_PORTAL, the OUT RPC maybe handled by MDT-side OUT RPC service thread unexpected, and replied via MDC_REPLY_PORTAL, instead of OSC_REPLY_PORTAL on which the OSP is waiting for the reply. Then caused the OSP-side OUT RPC timeout and resend again and again.

      The bad case also can happen when OSP wants to talk with MDT via OUT_PORTAL.

      Because NDE I has already used the OUT RPC for talking among MDTs. To be compatible with the old version, we cannot change the MDT-side OUT RPC reply portal. So we have to chance OST-side OUT RPC reply portal to "MDC_REPLY_PORTAL". But it is strange for OST-side to use MDT-side reply portal.

      2. The OUT RPC version is fixed on "LUSTRE_MDS_VERSION", in spite of the RPC is to MDT or to OST. Also confused others. We can re-define "tgt_out_handlers". But it may break the policy of Unified Target.

      3. Pack multiple idempotent sub-requests into single OUT RPC. In general, the OUT RPC should not assume that the sub-requests are related with each other. So even if one sub-request failed to be executed, the others should not be ignored. But in current implementation, it is not. If the other sub-requests are not related with the failed one, then such behavior is unexpected. Unfortunately, it is not easy to judge whether one sub-request is related with the others within current OUT request format, especially consider to be compatible with DNE I.

      4. Iteration via OUT. I found some client-side iteration framework in osp_md_object.c, but seems no server side handler. Do we have any plan to support that?

      Attachments

        Issue Links

          Activity

            [LU-4215] Some expected improvements for OUT

            Currently, LFSCK uses OUT RPC to talk with OST via OSP, it shares the interface out_prep_update_req() with the RPC to/from MDT. Inside such function, it always uses LUSTRE_MDS_VERSION in spite of whether it is for OST or MDT, which is confused.

            yong.fan nasf (Inactive) added a comment - Currently, LFSCK uses OUT RPC to talk with OST via OSP, it shares the interface out_prep_update_req() with the RPC to/from MDT. Inside such function, it always uses LUSTRE_MDS_VERSION in spite of whether it is for OST or MDT, which is confused.

            Hmm, I think #2 means we also pack OUT RPC with LUSTRE_MDS_VERSION (see out_prep_update_req), no matter this OUT RPC will be sent to MDS or OST. Right now, DNE only send out RPC to another MDS, but for LFSCK, I assume some OUT RPC needs to be sent to OST. So I think this is the one needs to be fixed. Though I guess the request is from LFSCK project, probably Fan Yong can confirm.

            di.wang Di Wang (Inactive) added a comment - Hmm, I think #2 means we also pack OUT RPC with LUSTRE_MDS_VERSION (see out_prep_update_req), no matter this OUT RPC will be sent to MDS or OST. Right now, DNE only send out RPC to another MDS, but for LFSCK, I assume some OUT RPC needs to be sent to OST. So I think this is the one needs to be fixed. Though I guess the request is from LFSCK project, probably Fan Yong can confirm.

            Di, could you clarify on #2 a bit please?

            bzzz Alex Zhuravlev added a comment - Di, could you clarify on #2 a bit please?

            Andreas, yes.

            bzzz Alex Zhuravlev added a comment - Andreas, yes.

            Alex, is there a chance for you to work on patches for 2.6 for the #2 and #3 items? Di already has far too many 2.6 blocker bugs to work on this, so if we want these changes then you are the best candidate to do the work.

            adilger Andreas Dilger added a comment - Alex, is there a chance for you to work on patches for 2.6 for the #2 and #3 items? Di already has far too many 2.6 blocker bugs to work on this, so if we want these changes then you are the best candidate to do the work.

            yes, the ability to continue processing in case of error is important for batched DESTROY's, for example. as for efficiency, I think we should not use obdo - it's huge, instead we should probably be able to get/set just a subset of attributes like dt_attr_set() allows.

            bzzz Alex Zhuravlev added a comment - yes, the ability to continue processing in case of error is important for batched DESTROY's, for example. as for efficiency, I think we should not use obdo - it's huge, instead we should probably be able to get/set just a subset of attributes like dt_attr_set() allows.

            For requirement 3, if we want to support packaged attr_get/xattr_get on multiple OST-objects via single OUT RPC, then we need to continue the OUT RPC handling even if some sub-requests failed. For example, the MDT (via OSP) wants to attr_get/xattr_get on both OST-object1 and OST-object2 via single OUT RPC, and it does not whether the two targets exists or not; so on the OST-side, it should not skip the OST-object2 even if OST-object1 does not exist.

            Currently, because we do not support that well yet, the MDT (via OSP) only pack the sub-requests which belong to the same target object is the OUT RPC. Once we improvement it, we can consider to make the OUT RPC to be more efficient.

            yong.fan nasf (Inactive) added a comment - For requirement 3, if we want to support packaged attr_get/xattr_get on multiple OST-objects via single OUT RPC, then we need to continue the OUT RPC handling even if some sub-requests failed. For example, the MDT (via OSP) wants to attr_get/xattr_get on both OST-object1 and OST-object2 via single OUT RPC, and it does not whether the two targets exists or not; so on the OST-side, it should not skip the OST-object2 even if OST-object1 does not exist. Currently, because we do not support that well yet, the MDT (via OSP) only pack the sub-requests which belong to the same target object is the OUT RPC. Once we improvement it, we can consider to make the OUT RPC to be more efficient.

            Andreas, nasf, only 1 and 4 are resolved right now. 1 has been landed to master. and 4 will be resolved in LU-4690.(http://review.whamcloud.com/#/c/9511/).
            I am not so sure about 3, for DNE, there are no strong requirement to tell which updates are failed on the client side, because right now, RPC are supposed to be executed in a single transaction, if 1 update is failed, then other updates inside the RPC are supposed to be undo right now. And for readonly updates, there is only 1 update per RPC. Probably LFSCK have more requirements here?

            2 can be fixed in 2.6 definitely, IMHO.

            di.wang Di Wang (Inactive) added a comment - Andreas, nasf, only 1 and 4 are resolved right now. 1 has been landed to master. and 4 will be resolved in LU-4690 .( http://review.whamcloud.com/#/c/9511/ ). I am not so sure about 3, for DNE, there are no strong requirement to tell which updates are failed on the client side, because right now, RPC are supposed to be executed in a single transaction, if 1 update is failed, then other updates inside the RPC are supposed to be undo right now. And for readonly updates, there is only 1 update per RPC. Probably LFSCK have more requirements here? 2 can be fixed in 2.6 definitely, IMHO.

            Di, Alex, can you please comment whether this bug can be closed? I think many of the improvements discussed here for the update RPC format were landed to master via http://review.whamcloud.com/7128 "LU-3539 protocol: Change UPDATE_OBJ RPC format". The OUT portal was changed via http://review.whamcloud.com/8390 "LU-3467 target: use osc_reply_portal for OUT services".

            Are there more changes that are still needed (which would be best to do in 2.6 while the protocol can be easily changed) or can it be closed?

            adilger Andreas Dilger added a comment - Di, Alex, can you please comment whether this bug can be closed? I think many of the improvements discussed here for the update RPC format were landed to master via http://review.whamcloud.com/7128 " LU-3539 protocol: Change UPDATE_OBJ RPC format". The OUT portal was changed via http://review.whamcloud.com/8390 " LU-3467 target: use osc_reply_portal for OUT services". Are there more changes that are still needed (which would be best to do in 2.6 while the protocol can be easily changed) or can it be closed?

            I also would like to make minor changes to the protocol:

            • a flag saying whether to interrupt handling up on an error or continue (useful for batched destroys)
            • more compact attr_set format to let many uid/gid changes in a single RPC (>bytes per object at the moment with struct obdo)
            bzzz Alex Zhuravlev added a comment - I also would like to make minor changes to the protocol: a flag saying whether to interrupt handling up on an error or continue (useful for batched destroys) more compact attr_set format to let many uid/gid changes in a single RPC (>bytes per object at the moment with struct obdo)

            For #3 there is the idea of "batchid" in the OUT request structure:

            struct update {
                    __u32           u_type;
                    __u32           u_batchid;
                    struct lu_fid   u_fid;
                    __u32           u_lens[UPDATE_BUF_COUNT];
                    __u32           u_bufs[0];
            };
            

            This allows the batched request to combine multiple updates into a single transaction by using the same "u_batchid", and updates with different "u_batchid" may be put into a separate transaction.

            I think that Di has some patch to change the OUT protocol a bit, though it doesn't really change the above semantic of using u_batchid to decide which updates belong in the same transaction. I'm not sure which patch of his this is, but it is intended to allow passing the master transno as the u_batchid. This would make the OUT protocol incompatible with older servers, but since it is currently only used between the MDTs this shouldn't be a big problem (they would need to be updated at the same time anyway).

            adilger Andreas Dilger added a comment - For #3 there is the idea of "batchid" in the OUT request structure: struct update { __u32 u_type; __u32 u_batchid; struct lu_fid u_fid; __u32 u_lens[UPDATE_BUF_COUNT]; __u32 u_bufs[0]; }; This allows the batched request to combine multiple updates into a single transaction by using the same "u_batchid", and updates with different "u_batchid" may be put into a separate transaction. I think that Di has some patch to change the OUT protocol a bit, though it doesn't really change the above semantic of using u_batchid to decide which updates belong in the same transaction. I'm not sure which patch of his this is, but it is intended to allow passing the master transno as the u_batchid. This would make the OUT protocol incompatible with older servers, but since it is currently only used between the MDTs this shouldn't be a big problem (they would need to be updated at the same time anyway).

            People

              bzzz Alex Zhuravlev
              yong.fan nasf (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated: