On Thu, Sep 6, 2018 at 7:44 PM, Sage Weil <sweil@xxxxxxxxxx> wrote:
> It includes everything that passes through the ENCODE_{START,FINISH}
> macros, which is pretty much every object with an encode/decode method
> defined.
>
> One could write a tool to pull data structures out of, say, bluestore, but
> that would only cover the dozen or so bluestore-related types.
> ceph-dencoder currently recognizes ~425. About 220 are captures by the
> most recent version in ceph-object-corpus (kraken :( ).
Ah right, I was thinking we could find all the stored other types
within that as well, but setting up something that understands the
metadata well enough to do so would be a pretty serious undertaking.
Hrm. :/
>
> Unfortunately the encode dump stuff is super inefficient.. I don't think
> it's something we can easily build in to our real builds. Well... maybe
> we could build it into the notcmalloc (debug) builds, actually...
I've no idea how this is actually implemented, but I'd imagine that a
config switch enabling it wouldn't be that expensive when turned off?
Certainly if it's an internal-only build like you mention.
On Thu, Sep 6, 2018 at 7:50 PM, Sage Weil <sweil@xxxxxxxxxx> wrote:
> On Thu, 6 Sep 2018, Noah Watkins wrote:
>> On Thu, Sep 6, 2018 at 4:30 PM Sage Weil <sweil@xxxxxxxxxx> wrote:
>> >
>> > The problem is that those generate_test_instances are (1) sparse and
>> > minimal, and (2) would require huge developer investment to fill in with
>> > "realistic" instances, and (3) even though wouldn't necessarily be
>>
>> If the goal (and maybe i'm missing the point in this thread) is to
>> check that old/new code can read new/old objects, what are some
>> examples of unrealistic instances that would throw a wrench in the
>> testing? The case that comes to mind is when decoding behavior is a
>> function of the content being decoded (beyond the standard struct
>> version info). If the testing extends to higher levels where the
>> relationship between values in a decoded object can be checked for
>> consistency / sanity then this makes sense but I didn't get that from
>> reading through the thread.
Yeah, a lot of the objects are just default-initialized or contain
only one or two entries in their maps. Whereas if we switch from using
a list to a map, or a map to a set, or using a map of pairs instead of
an ordered array...all that can easily introduce issues when trying to
encode the old version from our new in-memory state. This is good for
checking all those things, though indeed it doesn't cover the
higher-level "did we actually select the *right* version to encode"
steps.
>
> Right now the object corpus tests only verify that we can decode all prior
> versions. And there is a special way to indicate that an incompatible
> encoding change has been made (that tends to happens at once for most
> major releases it seems). And all the test is doing is verifying that
> we can decode without throwing an exception. Surprising it catches lots
> of careless errors.
>
> This could expand that to ensure that newer object instances are decodable
> by older versions. Mostly that just will just make sure the struct length
> stuff is working properly, so probably not likely to catch much.
IIRC it also compares the json dumps of structures to make sure they
show up the same across versions (or maybe it's through some
decode/encode step on each side to make sure they don't change?
Something, anyway.). That should be a pretty robust check, especially
within a stable series.
-Greg
> When encoding changes are feature dependent we could do more: we could
> extend the object corpus to include objects encoded with an older
> release's feature bits. That's a bit more involved.
>
> As is often the case, though, just because something should always work
> doesn't mean it will when you test it :). This probably isn't the
> highest-value time investment, but it does also help address the problem
> of forgetting to add objects from every release to the corpus (which is
> traditionally an annoying manual process), so I like it...
>
> sage
