Separating Peering from PG

Separating Peering from PG

[Loic Dachary]

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

Hi Sage,

In order to move the PG peering code out of PG.{cc,h} (which is the next step in refactoring PGs as suggested by Sam http://pad.ceph.com/p/Erasure_encoding_as_a_storage_backend ) I think it would be sensible to:

* Move PG::RecoveryStats in PGRecoveryStat.{cc,h}
* Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryStats
* Move Peering states / methods out of PGRecoveryStat.{cc,h} and into PGPeering.{cc,h} 
* Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface

Because this approach not only moves the peering out of PG.{cc,h} but also the rest of the state logic, I would like to know if this seems sensible to you. Also, introducing an abstract base class to help isolate the PG interface and facilitate writing fixtures has not been discussed yet. 

Cheers

-- 
Loïc Dachary, Artisan Logiciel Libre
All that is necessary for the triumph of evil is that good people do nothing.


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

Re: Separating Peering from PG

[Samuel Just]

I'm not sure I understand, by PG::RecoveryStats, do you mean PG::RecoveryState?
-Sam

On Fri, Jun 21, 2013 at 6:37 AM, Loic Dachary <loic@dachary.org> wrote:
> Hi Sage,
>
> In order to move the PG peering code out of PG.{cc,h} (which is the next step in refactoring PGs as suggested by Sam http://pad.ceph.com/p/Erasure_encoding_as_a_storage_backend ) I think it would be sensible to:
>
> * Move PG::RecoveryStats in PGRecoveryStat.{cc,h}
> * Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryStats
> * Move Peering states / methods out of PGRecoveryStat.{cc,h} and into PGPeering.{cc,h}
> * Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface
>
> Because this approach not only moves the peering out of PG.{cc,h} but also the rest of the state logic, I would like to know if this seems sensible to you. Also, introducing an abstract base class to help isolate the PG interface and facilitate writing fixtures has not been discussed yet.
>
> Cheers
>
> --
> Loïc Dachary, Artisan Logiciel Libre
> All that is necessary for the triumph of evil is that good people do nothing.
>
--
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

Re: Separating Peering from PG

[Loic Dachary]

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



On 06/21/2013 06:38 PM, Samuel Just wrote:
> I'm not sure I understand, by PG::RecoveryStats, do you mean PG::RecoveryState?

Yes I do, sorry for the confusion. Fixed the typos for clarity, hopefully.

* Move PG::RecoveryState in PGRecoveryState.{cc,h}
* Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryState
* Move Peering states / methods out of PGRecoveryState.{cc,h} and into PGPeering.{cc,h}
* Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface

Cheers

> -Sam
> 
> On Fri, Jun 21, 2013 at 6:37 AM, Loic Dachary <loic@dachary.org> wrote:
>> Hi Sage,
>>
>> In order to move the PG peering code out of PG.{cc,h} (which is the next step in refactoring PGs as suggested by Sam http://pad.ceph.com/p/Erasure_encoding_as_a_storage_backend ) I think it would be sensible to:
>>
>> * Move PG::RecoveryStats in PGRecoveryStat.{cc,h}
>> * Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryStats
>> * Move Peering states / methods out of PGRecoveryStat.{cc,h} and into PGPeering.{cc,h}
>> * Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface
>>
>> Because this approach not only moves the peering out of PG.{cc,h} but also the rest of the state logic, I would like to know if this seems sensible to you. Also, introducing an abstract base class to help isolate the PG interface and facilitate writing fixtures has not been discussed yet.
>>
>> Cheers
>>
>> --
>> Loïc Dachary, Artisan Logiciel Libre
>> All that is necessary for the triumph of evil is that good people do nothing.
>>
> --
> 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

-- 
Loïc Dachary, Artisan Logiciel Libre
All that is necessary for the triumph of evil is that good people do nothing.


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

Re: Separating Peering from PG

[Loic Dachary]

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

Hi Sam,

This draft commit may clarify what I mean

https://github.com/dachary/ceph/commit/5c6678385eed4f8769d0f5ee38ad629e32b902a6

Cheers

On 06/21/2013 07:10 PM, Loic Dachary wrote:
> 
> 
> On 06/21/2013 06:38 PM, Samuel Just wrote:
>> I'm not sure I understand, by PG::RecoveryStats, do you mean PG::RecoveryState?
> 
> Yes I do, sorry for the confusion. Fixed the typos for clarity, hopefully.
> 
> * Move PG::RecoveryState in PGRecoveryState.{cc,h}
> * Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryState
> * Move Peering states / methods out of PGRecoveryState.{cc,h} and into PGPeering.{cc,h}
> * Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface
> 
> Cheers
> 
>> -Sam
>>
>> On Fri, Jun 21, 2013 at 6:37 AM, Loic Dachary <loic@dachary.org> wrote:
>>> Hi Sage,
>>>
>>> In order to move the PG peering code out of PG.{cc,h} (which is the next step in refactoring PGs as suggested by Sam http://pad.ceph.com/p/Erasure_encoding_as_a_storage_backend ) I think it would be sensible to:
>>>
>>> * Move PG::RecoveryStats in PGRecoveryStat.{cc,h}
>>> * Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryStats
>>> * Move Peering states / methods out of PGRecoveryStat.{cc,h} and into PGPeering.{cc,h}
>>> * Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface
>>>
>>> Because this approach not only moves the peering out of PG.{cc,h} but also the rest of the state logic, I would like to know if this seems sensible to you. Also, introducing an abstract base class to help isolate the PG interface and facilitate writing fixtures has not been discussed yet.
>>>
>>> Cheers
>>>
>>> --
>>> Loïc Dachary, Artisan Logiciel Libre
>>> All that is necessary for the triumph of evil is that good people do nothing.
>>>
>> --
>> 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
> 

-- 
Loïc Dachary, Artisan Logiciel Libre
All that is necessary for the triumph of evil is that good people do nothing.


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

Re: Separating Peering from PG

[Loic Dachary]

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

Hi,

The refactor suggested below is implemented and compiles ( does not link yet ). 

https://github.com/dachary/ceph/commit/03b18a5f4b985781316e9aa9f1e73d3300b95a91
from the branch
https://github.com/dachary/ceph/tree/wip-5433

I hope it will help clarify my intentions. I created a task to track this work : http://tracker.ceph.com/issues/5433

Cheers

On 06/21/2013 07:51 PM, Loic Dachary wrote:
> Hi Sam,
> 
> This draft commit may clarify what I mean
> 
> https://github.com/dachary/ceph/commit/5c6678385eed4f8769d0f5ee38ad629e32b902a6
> 
> Cheers
> 
> On 06/21/2013 07:10 PM, Loic Dachary wrote:
>>
>>
>> On 06/21/2013 06:38 PM, Samuel Just wrote:
>>> I'm not sure I understand, by PG::RecoveryStats, do you mean PG::RecoveryState?
>>
>> Yes I do, sorry for the confusion. Fixed the typos for clarity, hopefully.
>>
>> * Move PG::RecoveryState in PGRecoveryState.{cc,h}
>> * Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryState
>> * Move Peering states / methods out of PGRecoveryState.{cc,h} and into PGPeering.{cc,h}
>> * Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface
>>
>> Cheers
>>
>>> -Sam
>>>
>>> On Fri, Jun 21, 2013 at 6:37 AM, Loic Dachary <loic@dachary.org> wrote:
>>>> Hi Sage,
>>>>
>>>> In order to move the PG peering code out of PG.{cc,h} (which is the next step in refactoring PGs as suggested by Sam http://pad.ceph.com/p/Erasure_encoding_as_a_storage_backend ) I think it would be sensible to:
>>>>
>>>> * Move PG::RecoveryStats in PGRecoveryStat.{cc,h}
>>>> * Create PGInterface : an abstract base class for PG enumerating all PG methods used by PGRecoveryStats
>>>> * Move Peering states / methods out of PGRecoveryStat.{cc,h} and into PGPeering.{cc,h}
>>>> * Write tests for PGPeering.{cc,h}, using a fixture derived from PGInterface
>>>>
>>>> Because this approach not only moves the peering out of PG.{cc,h} but also the rest of the state logic, I would like to know if this seems sensible to you. Also, introducing an abstract base class to help isolate the PG interface and facilitate writing fixtures has not been discussed yet.
>>>>
>>>> Cheers
>>>>
>>>> --
>>>> Loïc Dachary, Artisan Logiciel Libre
>>>> All that is necessary for the triumph of evil is that good people do nothing.
>>>>
>>> --
>>> 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
>>
> 

-- 
Loïc Dachary, Artisan Logiciel Libre
All that is necessary for the triumph of evil is that good people do nothing.


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