diff mbox

[08/39] mds: consider MDS as recovered when it reaches clientreply state.

Message ID 1363531902-24909-9-git-send-email-zheng.z.yan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yan, Zheng March 17, 2013, 2:51 p.m. UTC 
From: "Yan, Zheng" <zheng.z.yan@intel.com>

MDS in clientreply state already start servering requests. It also
make MDS::handle_mds_recovery() and MDS::recovery_done() match.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 src/mds/MDS.cc | 2 ++
 1 file changed, 2 insertions(+)

Comments

Gregory Farnum March 20, 2013, 6:40 p.m. UTC | #1 
The idea of this patch makes sense, but I'm not sure if we guarantee that each daemon sees every map update — if they don't then if an MDS misses the map moving an MDS into CLIENTREPLAY then they won't process them as having recovered on the next map. Sage or Joao, what are the guarantees subscription provides?  
-Greg

Software Engineer #42 @ http://inktank.com | http://ceph.com


On Sunday, March 17, 2013 at 7:51 AM, Yan, Zheng wrote:

> From: "Yan, Zheng" <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
>  
> MDS in clientreply state already start servering requests. It also
> make MDS::handle_mds_recovery() and MDS::recovery_done() match.
>  
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
> ---
> src/mds/MDS.cc (http://MDS.cc) | 2 ++
> 1 file changed, 2 insertions(+)
>  
> diff --git a/src/mds/MDS.cc (http://MDS.cc) b/src/mds/MDS.cc (http://MDS.cc)
> index 282fa64..b91dcbd 100644
> --- a/src/mds/MDS.cc (http://MDS.cc)
> +++ b/src/mds/MDS.cc (http://MDS.cc)
> @@ -1032,7 +1032,9 @@ void MDS::handle_mds_map(MMDSMap *m)
>  
> set<int> oldactive, active;
> oldmap->get_mds_set(oldactive, MDSMap::STATE_ACTIVE);
> + oldmap->get_mds_set(oldactive, MDSMap::STATE_CLIENTREPLAY);
> mdsmap->get_mds_set(active, MDSMap::STATE_ACTIVE);
> + mdsmap->get_mds_set(active, MDSMap::STATE_CLIENTREPLAY);
> for (set<int>::iterator p = active.begin(); p != active.end(); ++p)  
> if (*p != whoami && // not me
> oldactive.count(*p) == 0) // newly so?
> --  
> 1.7.11.7



--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gregory Farnum March 20, 2013, 7:09 p.m. UTC | #2 
Oh, also: s/clientreply/clientreplay in the commit message 

Software Engineer #42 @ http://inktank.com | http://ceph.com


On Sunday, March 17, 2013 at 7:51 AM, Yan, Zheng wrote:

> From: "Yan, Zheng" <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
> 
> MDS in clientreply state already start servering requests. It also
> make MDS::handle_mds_recovery() and MDS::recovery_done() match.
> 
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
> ---
> src/mds/MDS.cc (http://MDS.cc) | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/src/mds/MDS.cc (http://MDS.cc) b/src/mds/MDS.cc (http://MDS.cc)
> index 282fa64..b91dcbd 100644
> --- a/src/mds/MDS.cc (http://MDS.cc)
> +++ b/src/mds/MDS.cc (http://MDS.cc)
> @@ -1032,7 +1032,9 @@ void MDS::handle_mds_map(MMDSMap *m)
> 
> set<int> oldactive, active;
> oldmap->get_mds_set(oldactive, MDSMap::STATE_ACTIVE);
> + oldmap->get_mds_set(oldactive, MDSMap::STATE_CLIENTREPLAY);
> mdsmap->get_mds_set(active, MDSMap::STATE_ACTIVE);
> + mdsmap->get_mds_set(active, MDSMap::STATE_CLIENTREPLAY);
> for (set<int>::iterator p = active.begin(); p != active.end(); ++p) 
> if (*p != whoami && // not me
> oldactive.count(*p) == 0) // newly so?
> -- 
> 1.7.11.7



--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yan, Zheng March 21, 2013, 2:22 a.m. UTC | #3 
On 03/21/2013 02:40 AM, Greg Farnum wrote:
> The idea of this patch makes sense, but I'm not sure if we guarantee that each daemon sees every map update — if they don't then if an MDS misses the map moving an MDS into CLIENTREPLAY then they won't process them as having recovered on the next map. Sage or Joao, what are the guarantees subscription provides?  
> -Greg

See MDS::active_start(), it also kicks clientreplay waiters. And I will fix the 'clientreply' typo in my git tree.

Thanks
Yan, Zheng

> 
> Software Engineer #42 @ http://inktank.com | http://ceph.com
> 
> 
> On Sunday, March 17, 2013 at 7:51 AM, Yan, Zheng wrote:
> 
>> From: "Yan, Zheng" <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
>>  
>> MDS in clientreply state already start servering requests. It also
>> make MDS::handle_mds_recovery() and MDS::recovery_done() match.
>>  
>> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
>> ---
>> src/mds/MDS.cc (http://MDS.cc) | 2 ++
>> 1 file changed, 2 insertions(+)
>>  
>> diff --git a/src/mds/MDS.cc (http://MDS.cc) b/src/mds/MDS.cc (http://MDS.cc)
>> index 282fa64..b91dcbd 100644
>> --- a/src/mds/MDS.cc (http://MDS.cc)
>> +++ b/src/mds/MDS.cc (http://MDS.cc)
>> @@ -1032,7 +1032,9 @@ void MDS::handle_mds_map(MMDSMap *m)
>>  
>> set<int> oldactive, active;
>> oldmap->get_mds_set(oldactive, MDSMap::STATE_ACTIVE);
>> + oldmap->get_mds_set(oldactive, MDSMap::STATE_CLIENTREPLAY);
>> mdsmap->get_mds_set(active, MDSMap::STATE_ACTIVE);
>> + mdsmap->get_mds_set(active, MDSMap::STATE_CLIENTREPLAY);
>> for (set<int>::iterator p = active.begin(); p != active.end(); ++p)  
>> if (*p != whoami && // not me
>> oldactive.count(*p) == 0) // newly so?
>> --  
>> 1.7.11.7
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gregory Farnum March 21, 2013, 9:43 p.m. UTC | #4 
On Wed, Mar 20, 2013 at 7:22 PM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
> On 03/21/2013 02:40 AM, Greg Farnum wrote:
>> The idea of this patch makes sense, but I'm not sure if we guarantee that each daemon sees every map update — if they don't then if an MDS misses the map moving an MDS into CLIENTREPLAY then they won't process them as having recovered on the next map. Sage or Joao, what are the guarantees subscription provides?
>> -Greg
>
> See MDS::active_start(), it also kicks clientreplay waiters. And I will fix the 'clientreply' typo in my git tree.

That's for the recovering MDS — I was referring to observers who are
watching it come up. However, I've just checked and the monitor does
guarantee every-map delivery, so this isn't a problem.

Reviewed-by: Greg Farnum <greg@inktank.com>

>
> Thanks
> Yan, Zheng
>
>>
>> Software Engineer #42 @ http://inktank.com | http://ceph.com
>>
>>
>> On Sunday, March 17, 2013 at 7:51 AM, Yan, Zheng wrote:
>>
>>> From: "Yan, Zheng" <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
>>>
>>> MDS in clientreply state already start servering requests. It also
>>> make MDS::handle_mds_recovery() and MDS::recovery_done() match.
>>>
>>> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
>>> ---
>>> src/mds/MDS.cc (http://MDS.cc) | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/src/mds/MDS.cc (http://MDS.cc) b/src/mds/MDS.cc (http://MDS.cc)
>>> index 282fa64..b91dcbd 100644
>>> --- a/src/mds/MDS.cc (http://MDS.cc)
>>> +++ b/src/mds/MDS.cc (http://MDS.cc)
>>> @@ -1032,7 +1032,9 @@ void MDS::handle_mds_map(MMDSMap *m)
>>>
>>> set<int> oldactive, active;
>>> oldmap->get_mds_set(oldactive, MDSMap::STATE_ACTIVE);
>>> + oldmap->get_mds_set(oldactive, MDSMap::STATE_CLIENTREPLAY);
>>> mdsmap->get_mds_set(active, MDSMap::STATE_ACTIVE);
>>> + mdsmap->get_mds_set(active, MDSMap::STATE_CLIENTREPLAY);
>>> for (set<int>::iterator p = active.begin(); p != active.end(); ++p)
>>> if (*p != whoami && // not me
>>> oldactive.count(*p) == 0) // newly so?
>>> --
>>> 1.7.11.7
>>
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/mds/MDS.cc b/src/mds/MDS.cc
index 282fa64..b91dcbd 100644
--- a/src/mds/MDS.cc
+++ b/src/mds/MDS.cc
@@ -1032,7 +1032,9 @@  void MDS::handle_mds_map(MMDSMap *m)
 
     set<int> oldactive, active;
     oldmap->get_mds_set(oldactive, MDSMap::STATE_ACTIVE);
+    oldmap->get_mds_set(oldactive, MDSMap::STATE_CLIENTREPLAY);
     mdsmap->get_mds_set(active, MDSMap::STATE_ACTIVE);
+    mdsmap->get_mds_set(active, MDSMap::STATE_CLIENTREPLAY);
     for (set<int>::iterator p = active.begin(); p != active.end(); ++p) 
       if (*p != whoami &&            // not me
 	  oldactive.count(*p) == 0)  // newly so?