Bug 1205186 - RCU changes wrt peers to be done for GlusterFS-3.7.0
Summary: RCU changes wrt peers to be done for GlusterFS-3.7.0
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: glusterd
Version: mainline
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: ---
Assignee: Kaushal
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1191030 1211207 1218031
TreeView+ depends on / blocked
 
Reported: 2015-03-24 12:24 UTC by Kaushal
Modified: 2022-05-12 12:45 UTC (History)
6 users (show)

Fixed In Version: glusterfs-3.8rc2
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-06-16 12:45:03 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Kaushal 2015-03-24 12:24:58 UTC
Completing the RCU changes for GlusterFS-3.7.0  requires the following changes to be done,
1. Handle possible deadlock issues due to glusterd_quorum_count macro
2. Handle possible deadlock issues in glusterd_friend_remove
3. Changes needed in the volume operation transaction system to be compliant with the peer RCU changes. (particularly the transaction peer lists)

Comment 1 Anand Avati 2015-03-24 12:30:50 UTC
REVIEW: http://review.gluster.org/9978 (glusterd: Prevent possible deadlock due to glusterd_quorum_count) posted (#3) for review on master by Kaushal M (kaushal)

Comment 2 Anand Avati 2015-03-24 12:30:58 UTC
REVIEW: http://review.gluster.org/9979 (glusterd: Prevent possible dealock in glusterd_friend_remove) posted (#2) for review on master by Kaushal M (kaushal)

Comment 3 Anand Avati 2015-03-25 11:36:10 UTC
COMMIT: http://review.gluster.org/9978 committed in master by Krishnan Parthasarathi (kparthas) 
------
commit 891c7d0fad16e7f0f51af217a8aad0a653a353d7
Author: Kaushal M <kaushal>
Date:   Tue Mar 24 12:15:19 2015 +0530

    glusterd: Prevent possible deadlock due to glusterd_quorum_count
    
    Also rename the macro glusterd_quorum_count to GLUSTERD_QUORUM_COUNT so
    that it stands out as a macro.
    
    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      0fbd7ba Prevent possible deadlock due to glusterd_quorum_count
      5da3062 Rename glusterd_quorum_count to GLUSTERD_QUORUM_COUNT
      b3aa3c4 Enclose GLUSTERD_QUORUM_COUNT definition in a do-while block
    
    [1]: https://github.com/kshlm/glusterfs/tree/urcu
    
    Change-Id: Ic4b3949f303d72ce53e0139f62b83b8d13fb4e47
    BUG: 1205186
    Signed-off-by: Kaushal M <kaushal>
    Reviewed-on: http://review.gluster.org/9978
    Reviewed-by: Atin Mukherjee <amukherj>
    Reviewed-by: Gaurav Kumar Garg <ggarg>
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Krishnan Parthasarathi <kparthas>
    Tested-by: Krishnan Parthasarathi <kparthas>

Comment 4 Anand Avati 2015-03-25 11:37:02 UTC
COMMIT: http://review.gluster.org/9979 committed in master by Krishnan Parthasarathi (kparthas) 
------
commit 91708968d01b253ee1ebcec1fdc3d602435144ef
Author: Kaushal M <kaushal>
Date:   Tue Mar 24 12:56:09 2015 +0530

    glusterd: Prevent possible dealock in glusterd_friend_remove
    
    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      b02290e Prevent possible dealock in glusterd_friend_remove
    
    [1]: https://github.com/kshlm/glusterfs/tree/urcu
    
    Change-Id: I1efeaf18f2054f4252ee95244908613542d209d9
    BUG: 1205186
    Signed-off-by: Kaushal M <kaushal>
    Reviewed-on: http://review.gluster.org/9979
    Reviewed-by: Atin Mukherjee <amukherj>
    Reviewed-by: Gaurav Kumar Garg <ggarg>
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Krishnan Parthasarathi <kparthas>
    Tested-by: Krishnan Parthasarathi <kparthas>

Comment 5 Anand Avati 2015-04-07 09:25:09 UTC
REVIEW: http://review.gluster.org/10147 (glusterd: Replace transaction peers lists) posted (#1) for review on master by Kaushal M (kaushal)

Comment 6 Anand Avati 2015-04-07 14:24:43 UTC
REVIEW: http://review.gluster.org/10147 (glusterd: Replace transaction peers lists) posted (#2) for review on master by Kaushal M (kaushal)

Comment 7 Anand Avati 2015-04-08 11:32:09 UTC
REVIEW: http://review.gluster.org/10147 (glusterd: Replace transaction peers lists) posted (#3) for review on master by Kaushal M (kaushal)

Comment 8 Anand Avati 2015-04-09 04:57:46 UTC
REVIEW: http://review.gluster.org/10147 (glusterd: Replace transaction peers lists) posted (#4) for review on master by Kaushal M (kaushal)

Comment 9 Anand Avati 2015-04-10 09:59:33 UTC
REVIEW: http://review.gluster.org/10147 (glusterd: Replace transaction peers lists) posted (#5) for review on master by Kaushal M (kaushal)

Comment 10 Anand Avati 2015-04-10 09:59:36 UTC
REVIEW: http://review.gluster.org/10192 (glusterd: Remove direct references to peerinfo in frame cookies) posted (#1) for review on master by Kaushal M (kaushal)

Comment 11 Anand Avati 2015-04-13 06:32:03 UTC
COMMIT: http://review.gluster.org/10147 committed in master by Krishnan Parthasarathi (kparthas) 
------
commit 1efa50861b2cee68de9c9b751d9fc5eed08f5e5b
Author: Kaushal M <kaushal>
Date:   Thu Mar 26 15:18:54 2015 +0530

    glusterd: Replace transaction peers lists
    
    Transaction peer lists were used in GlusterD to peers belonging to a
    transaction. This was needed to prevent newly added peers performing
    partial transactions, which could be incorrect.
    
    This was accomplished by creating a seperate transaction peers list at
    the beginning of every transaction. A transaction peers list referenced
    the peerinfo data structures of the peers which were present at the
    beginning of the transaction. RCU protection of peerinfos referenced by
    the transaction peers list is a hard problem and difficult to do
    correctly.
    
    To have proper RCU protection of peerinfos, the transaction peers lists
    have been replaced by an alternative method to identify peers that
    belong to a transaction. The alternative method is to the global peers
    list along with generation numbers to identify peers that should belong
    to a transaction.
    
    This change introduces a global peer list generation number, and a
    generation number for each peerinfo object. Whenever a peerinfo object
    is created, the global generation number is bumped, and the peerinfos
    generation number is set to the bumped global generation.
    
    With the above changes, the algorithm to identify peers belonging to a
    transaction with RCU protection is as follows,
    - At the beginning of a transaction, the current global generation
      number is saved
    - To identify if a peers belonging to the transaction,
      - Start a RCU read critical section
      - For each peer in the global peers list,
        - If the peers generation number is not greater than the saved
          generation number, continue with the action on the peer
      - End the RCU read critical section
    
    The above algorithm guarantees that,
    - The peer list is not modified when a transaction is iterating through
      it
    - The transaction actions are only done on peers that were present when
      the transaction started
    
    But, as a transaction could iterate over the peers list multiple times,
    the algorithm cannot guarantee that same set of peers will be selected
    every time. A peer could get deleted between two iterations of the list
    within a transaction. This problem existed with transaction peers list
    as well, but unlike before now it will not lead to invalid memory access
    and potential crashes. This problem will be addressed seprately.
    
    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      52ded5b Add timespec_cmp
      44aedd8 Add create timestamp to peerinfo
      7bcbea5 Fix some silly mistakes
      13e3241 Add start time to opinfo
      17a6727 Use timestamp comparisions to identify xaction peers instead
              of a xaction peer list
      3be05b6 Correct check for peerinfo age
      70d5b58 Use read-critical sections for peer list iteration
      ba4dbca Use peerinfo timestamp checks in op-sm instead of xaction peer
              list
      d63f811 Add more peer status checks when iterating peers list in
              glusterd-syncop
      1998a2a Timestamp based peer list traversal of mgmtv3 xactions
      f3c1a42 Remove transaction peer lists
      b8b08ee Remove unused labels
      32e5f5b Remove 'npeers' usage
      a075fb7 Remove 'npeers' from mgmt-v3 framework
      12c9df2 Use generation number instead of timestamps.
      9723021 Remove timespec_cmp
      80ae2c6 Remove timespec.h include
      a9479b0 Address review comments on 10147/4
    
    [1]: https://github.com/kshlm/glusterfs/tree/urcu
    
    Change-Id: I9be1033525c0a89276f5b5d83dc2eb061918b97f
    BUG: 1205186
    Signed-off-by: Kaushal M <kaushal>
    Reviewed-on: http://review.gluster.org/10147
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Atin Mukherjee <amukherj>
    Reviewed-by: Anand Nekkunti <anekkunt>
    Reviewed-by: Krishnan Parthasarathi <kparthas>
    Tested-by: Krishnan Parthasarathi <kparthas>

Comment 12 Anand Avati 2015-04-13 09:50:38 UTC
REVIEW: http://review.gluster.org/10192 (glusterd: Remove direct references to peerinfo in frame cookies) posted (#2) for review on master by Kaushal M (kaushal)

Comment 13 Anand Avati 2015-04-22 06:53:38 UTC
REVIEW: http://review.gluster.org/10192 (glusterd: Remove direct references to peerinfo in frame cookies) posted (#3) for review on master by Kaushal M (kaushal)

Comment 14 Anand Avati 2015-04-23 08:23:05 UTC
REVIEW: http://review.gluster.org/10192 (glusterd: Remove direct references to peerinfo in frame cookies) posted (#4) for review on master by Kaushal M (kaushal)

Comment 15 Anand Avati 2015-04-27 05:39:07 UTC
REVIEW: http://review.gluster.org/10399 (glusterd: Remove direct references to peerinfo in frame cookies) posted (#1) for review on release-3.7 by Kaushal M (kaushal)

Comment 16 Anand Avati 2015-04-27 06:47:06 UTC
COMMIT: http://review.gluster.org/10192 committed in master by Krishnan Parthasarathi (kparthas) 
------
commit a1de3b05b3fd7514dbce5182c371c6be97819969
Author: Kaushal M <kaushal>
Date:   Fri Apr 10 13:02:59 2015 +0530

    glusterd: Remove direct references to peerinfo in frame cookies
    
    RCU protection requires that we don't have  direct references to
    protected data structures outside read-critical sections
    
    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      82ebfdd Remove direct references to peerinfo in frame cookies
      dec4bec Remove incorrect and unneeded code from
              gd_syncop_mgmt_v3_unlock_cbk_fn
      7aced7b Use stack allocated uuid for frame cookie.
      38e4124 Address comments from 10192/2
    
    [1]: https://github.com/kshlm/glusterfs/tree/urcu
    
    Change-Id: Ic50e5fca0be72af5090f4cf318efa55d29075de9
    BUG: 1205186
    Signed-off-by: Kaushal M <kaushal>
    Reviewed-on: http://review.gluster.org/10192
    Reviewed-by: Atin Mukherjee <amukherj>
    Tested-by: NetBSD Build System
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Krishnan Parthasarathi <kparthas>
    Tested-by: Krishnan Parthasarathi <kparthas>

Comment 17 Anand Avati 2015-04-28 12:49:11 UTC
COMMIT: http://review.gluster.org/10399 committed in release-3.7 by Krishnan Parthasarathi (kparthas) 
------
commit bb86ba720aa9a89b0d2df5f2a6ac65c87edd260b
Author: Kaushal M <kaushal>
Date:   Fri Apr 10 13:02:59 2015 +0530

    glusterd: Remove direct references to peerinfo in frame cookies
    
    RCU protection requires that we don't have  direct references to
    protected data structures outside read-critical sections
    
    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      82ebfdd Remove direct references to peerinfo in frame cookies
      dec4bec Remove incorrect and unneeded code from
              gd_syncop_mgmt_v3_unlock_cbk_fn
      7aced7b Use stack allocated uuid for frame cookie.
      38e4124 Address comments from 10192/2
    
    [1]: https://github.com/kshlm/glusterfs/tree/urcu
    
    Change-Id: Ic50e5fca0be72af5090f4cf318efa55d29075de9
    BUG: 1205186
    Signed-off-by: Kaushal M <kaushal>
    Reviewed-on: http://review.gluster.org/10399
    Reviewed-by: Krishnan Parthasarathi <kparthas>
    Tested-by: Krishnan Parthasarathi <kparthas>

Comment 18 Anand Avati 2015-04-28 13:19:17 UTC
REVIEW: http://review.gluster.org/10425 (glusterd: Use uint32_t for peerinfo generation number) posted (#1) for review on master by Kaushal M (kaushal)

Comment 19 Anand Avati 2015-04-28 13:21:45 UTC
REVIEW: http://review.gluster.org/10426 (glusterd: Use uint32_t for peerinfo generation number) posted (#1) for review on release-3.7 by Kaushal M (kaushal)

Comment 20 Anand Avati 2015-04-29 07:35:56 UTC
REVIEW: http://review.gluster.org/10425 (glusterd: Use uint32_t for peerinfo generation number) posted (#2) for review on master by Kaushal M (kaushal)

Comment 21 Anand Avati 2015-04-29 07:40:30 UTC
REVIEW: http://review.gluster.org/10426 (glusterd: Use uint32_t for peerinfo generation number) posted (#2) for review on release-3.7 by Kaushal M (kaushal)

Comment 22 Anand Avati 2015-04-30 08:23:59 UTC
COMMIT: http://review.gluster.org/10425 committed in master by Krishnan Parthasarathi (kparthas) 
------
commit 3d0c87a38802b0751c79ec0f11bb7f8972f417cd
Author: Kaushal M <kaushal>
Date:   Tue Apr 28 18:42:41 2015 +0530

    glusterd: Use uint32_t for peerinfo generation number
    
    Using a uint64_t for the peerinfo generation number was overkill for how
    the generation number is used within GlusterD. It also prevented
    GlusterD from running on 32-bit architechtures, as uatomic_add_return
    doesn't support 64-bit values on 32-bit architechtures.
    
    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      b78dba4 Use 32-bit generation number
      2c37e4b Change other generation number variables to uint32_t
    
    [1]: https://github.com/kshlm/glusterfs/tree/urcu
    
    Change-Id: I0f310f56a4fb97d6bcbc23255a379ed5bb1ed9e1
    BUG: 1205186
    Signed-off-by: Kaushal M <kaushal>
    Reviewed-on: http://review.gluster.org/10425
    Reviewed-by: Anand Nekkunti <anekkunt>
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Emmanuel Dreyfus <manu>
    Tested-by: Emmanuel Dreyfus <manu>
    Reviewed-by: Atin Mukherjee <amukherj>
    Reviewed-by: Krishnan Parthasarathi <kparthas>
    Tested-by: Krishnan Parthasarathi <kparthas>

Comment 23 Niels de Vos 2015-05-22 09:07:31 UTC
Some of the patches linked with this bug are only available in the master branch. Other bugs that this one blocks were used for backporting those changes.

Comment 24 Kaushal 2015-12-04 08:36:56 UTC
All the changes for this bug have been merged, and have been available since glusterfs-v3.7.0.

Comment 25 Niels de Vos 2016-06-16 12:45:03 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.8.0, please open a new bug report.

glusterfs-3.8.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://blog.gluster.org/2016/06/glusterfs-3-8-released/
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user


Note You need to log in before you can comment on or make changes to this bug.