Add a converter in OSDMap to split the ruleset into rule

Add a converter in OSDMap to split the ruleset into rule

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

Hi,

I've added a few comments inline at https://github.com/xiaoxichen/ceph/commit/354c09131a64ac1e1a67c71794d1a3bab8334ca8 . Could you explain in pseudo code, in the commit message, what upgrade strategy you are trying to implement ? 

For testing purposes, I would create border cases (i.e. having a rule with ruleid != ruleset) in a script similar to 

   https://github.com/ceph/ceph/blob/firefly/src/test/mon/osd-crush.sh

and then trigger the upgrade and check that it enforces the constraint. 

There is a need for a change near 

   https://github.com/ceph/ceph/blob/master/src/crush/CrushWrapper.cc#L787

so that it does not pick a ruleset different from the ruleid. Maybe that could be a first step in the right direction ? I mean, not modifying the existing maps yet, just enforcing ruleid == ruleset by changing the code. So that a new cluster does not suffer from this inconsistency.

Cheers

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Add a converter in OSDMap to split the ruleset into rule

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]



On 15/08/2014 11:20, Loic Dachary wrote:
> Hi,
> 
> I've added a few comments inline at https://github.com/xiaoxichen/ceph/commit/354c09131a64ac1e1a67c71794d1a3bab8334ca8 . Could you explain in pseudo code, in the commit message, what upgrade strategy you are trying to implement ? 
> 
> For testing purposes, I would create border cases (i.e. having a rule with ruleid != ruleset) in a script similar to 
> 
>    https://github.com/ceph/ceph/blob/firefly/src/test/mon/osd-crush.sh
> 
> and then trigger the upgrade and check that it enforces the constraint. 

Which is exactly what you did ;-)

> 
> There is a need for a change near 
> 
>    https://github.com/ceph/ceph/blob/master/src/crush/CrushWrapper.cc#L787
> 
> so that it does not pick a ruleset different from the ruleid. Maybe that could be a first step in the right direction ? I mean, not modifying the existing maps yet, just enforcing ruleid == ruleset by changing the code. So that a new cluster does not suffer from this inconsistency.
> 
> Cheers
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Add a converter in OSDMap to split the ruleset into rule

[Chen, Xiaoxi]

在 2014-8-15，17:26，"Loic Dachary" <loic@dachary.org> 写道：

> 
> 
> On 15/08/2014 11:20, Loic Dachary wrote:
>> Hi,
>> 
>> I've added a few comments inline at https://github.com/xiaoxichen/ceph/commit/354c09131a64ac1e1a67c71794d1a3bab8334ca8 . Could you explain in pseudo code, in the commit message, what upgrade strategy you are trying to implement 

Yes, will do so.
>> 
>> For testing purposes, I would create border cases (i.e. having a rule with ruleid != ruleset) in a script similar to 
>> 
>>   https://github.com/ceph/ceph/blob/firefly/src/test/mon/osd-crush.sh
>> 
>> and then trigger the upgrade and check that it enforces the constraint.
> 
> Which is exactly what you did ;-)
> 
>> 
>> There is a need for a change near 
>> 
>>   https://github.com/ceph/ceph/blob/master/src/crush/CrushWrapper.cc#L787
>> 
>> so that it does not pick a ruleset different from the ruleid. Maybe that could be a first step in the right direction ? I mean, not modifying the existing maps yet, just enforcing ruleid == ruleset by changing the code. So that a new cluster does not suffer from this inconsistency.
>> 

originally I plan to do this after the converter, but it is a simple change, will make it right now. 

Thanks a lot for your review and will issue a pull request after refine this patch
>> Cheers
> 
> -- 
> Loïc Dachary, Artisan Logiciel Libre
>