A good place to start would be to get some unit test coverage of the PG::PriorSet logic. It's already factored out to a degree. osd_types.h::pg_interval_t::check_new_interval would also be a good place for testing. For something rather more ambitious, the merge_log/rewind_dievergent_log and related methods could use better test coverage, but that would require some significant refactoring. -Sam On Thu, Apr 25, 2013 at 1:20 PM, Loic Dachary <loic@xxxxxxxxxxx> wrote: > > > On 04/25/2013 09:47 PM, Gregory Farnum wrote: >> On Thu, Apr 25, 2013 at 11:28 AM, Loic Dachary <loic@xxxxxxxxxxx> wrote: >>> >>> >>> On 04/25/2013 06:45 PM, Gregory Farnum wrote: >>>> On Thu, Apr 25, 2013 at 1:17 AM, Loic Dachary <loic@xxxxxxxxxxx> wrote: >>>>> Hi, >>>>> >>>>> In the context of the implementation of >>>>> >>>>> http://wiki.ceph.com/01Planning/02Blueprints/Dumpling/Erasure_encoding_as_a_storage_backend >>>>> >>>>> I'm preparing tests to assert that the modifications that will be made to the existing PG and ReplicatedPG will not introduce regressions. Unless I'm mistaken, there are no unit tests or functional tests for PG or ReplicatedPG at the moment. I thought that it might be useful to add an abstract OSD class from which OSD is derived, to allow a test to derive from it to implement synthetic behavior. >>>>> >>>>> >>>>> I gave it a try and it passes make check successfully. It is a little hairy because I commented out the code instead of removing it to help with rebasing. >>>>> >>>>> https://github.com/dachary/ceph/commit/a9eb690a3537c3f844b47e76cde048b084e314eb >>>>> >>>>> I'll now try to use it to implement some tests for ReplicatedPG. >>>>> >>>>> I would very much appreciate if someone has an advice on the best way to proceed. Even if it's to say : "you're crazy, don't go there ! you're wasting your time, there is a much simpler way !" ;-) >>>> >>>> What's your plot here? The OSD is fairly large to be doing unit tests >>>> on (although it's not insensible) — but if we want to form a new PG it >>>> might be best to start out by making abstract PG implementations. >>>> >>> >>> My reasoning was that to for a new PG it was enough to rely on >>> >>> https://github.com/ceph/ceph/blob/master/src/osd/PG.h >>> >>> and create a new ErasureEncodingPG.{cc,h} after >>> >>> https://github.com/ceph/ceph/blob/master/src/osd/ReplicatedPG.cc >>> >>> Since ReplicatedPG is the only class derived from PG, I assumed there will be a need to move code from ReplicatedPG to PG and vice versa to make sure PG can actually be the base class of two different implementations. Are you suggesting that PG should be abstracted ? Is there a reason why it would not be a good base class for a new ErasureEncodedPG ? >> >> Oh, it *should* be the base class, but the distinction between PG and >> ReplicatedPG right now is...vanishing. I had a quick chat about this >> with Sam a couple days ago and we agreed the first task here is going >> to be properly slicing them up somehow (the current state of affairs >> is not a good model, even, from what he's told me). > > Understood :-) I'm quite sure this is out of my league. > >> I'm not saying the OSD is a bad plan, just that I'm not sure what kind >> of useful tests we could produce by abstracting it — whereas the unit >> tests we can write against a PG seem a lot simpler and more obvious. I >> could well be missing something, though. > > I'll focus on writing PG tests. It will help me understand the details of PG / ReplicatedPG and eventually help with the necessary refactoring. > > Thanks again for the guidance :-) > >> -Greg >> Software Engineer #42 @ http://inktank.com | http://ceph.com > > -- > Loïc Dachary, Artisan Logiciel Libre > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
