Re: Brooklyn persisted state: guidelines for writing entities [check-state utility]

[Aled Sage <al...@gmail.com>]

All (especially Svet!),

I'm looking at adding a `brooklyn check-state --persistenceDir ...` utility.

This would do two things:

 1. make sure that the state is rebindable;
 2. make sure the state follows best practices (e.g. no non-static
    classes, no inner classes, avoid use of @SetFromFlag where possible,
    etc)

For (1), I hope we can use the HOT_BACKUP code-path to check this.

For (2), there are a few possibilities. We could either run a set of 
checks against the persisted Strings (i.e. against the XML itself). Or 
we could de-serialize the objects and run a set of checks against that.

I think I favour validating against the XML - we can search for 
particular patterns. If doing it against the Java objects, we'd need to 
reflectively walk the object hierarchy to see if anything looks bad.

Thoughts/opinions?

Aled

On 10/03/2015 17:29, Aled Sage wrote:
> All,
>
> I've made some improvements to the docs at [1], to describe best 
> practices. Any review / feedback extremely welcome.
>
> Aled
>
> [1] https://github.com/apache/incubator-brooklyn/pull/548
>
>
> On 02/03/2015 15:43, Aled Sage wrote:
>> Hi all,
>>
>> We could do with better guidelines for folk writing entities, so that 
>> it produces "good" persisted state.
>>
>> By "good", I mean that it will persist and rebind successfully, and 
>> that persisted state will not be overly affected by code changes in 
>> future versions of the relevant entities.
>>
>> I suggest we add to the section "Writing Persistable Code" in [1], 
>> and that we move this to somewhere more prominent.
>>
>> ---
>> I wanted to get the community's input on the overall approach, and 
>> any thoughts on the guidelines.
>>
>> We can switch to PR reviews/comments when it gets into the low-level 
>> details.
>>
>> _*Current implementation*_
>> We use xstream to persist/deserialize the Java objects as XML. For an 
>> entity, we persist its attributes and config. We also persist the 
>> policies, enrichers and feeds associated with the entity. We also 
>> persist locations + catalog.
>>
>> The XML serialized form includes the class name and also the field 
>> names as the XML tags.
>>
>> We may switch to json for the persistence format. This would give two 
>> benefits:
>>
>>  * Better persistence to things like MongoDB and CouchBase.
>>  * Move towards aligning with our YAML format (used for blueprint
>>    definition), for improved consistency; eventually persisted state
>>    could be deployed to a new Brooklyn!
>>
>>
>> _*Additional guidelines*_
>>
>> 1. Never use anonymous inner classes.
>>    reason: the MyOuterClass$1 auto-generated name is really brittle,
>>    for if the code is changed in the future.
>> 2. Never use non-static inner classes.
>>    reason: the outer class needs to be serialized as well.
>> 3. Always use sensible field names (and use `transient` whenever you
>>    don't want it persisted).
>>    reason: the field name will be part of the persisted state.
>> 4. Consider using Value Objects for persisted values.
>>    reason: can give clearer separation of responsibilities, and clearer
>>    control of what fields are being persisted (making backwards
>>    compatibility simpler).
>> 5. Consider writing transformers to handle backwards-incompatible code
>>    changes.
>>    reason: Brooklyn supports applying transformations to the persisted
>>    state, which can be done as part of an upgrade process.
>>
>>
>> _*Additional features?
>> *_There are a few additional features we could consider supporting:
>>
>> 1. Enforcement of guidelines
>>    e.g. have a CLI option for `brooklyn check-state --strict`. This
>>    would ensure guidelines are followed and that rebind will be 
>> possible.
>> 2. Improved error reporting, including in web-console, when there are
>>    rebind problems.
>> 3. Support toJson() / fromJson() on objects being serialized, so that
>>    folk can control their own custom serialization.
>> 4. Support JavaBeans styles (i.e. persisting based on the getter method
>>    name, rather than the field names).
>>
>> For (3) and (4) I'm hesitant for us to support more stuff (more ways 
>> of doing things) unless it's going to genuinely be of major benefit 
>> to users.
>>
>> Aled
>>
>> [1] 
>> https://brooklyn.incubator.apache.org/v/latest/ops/persistence/index.html
>>
>>
>

Re: Brooklyn persisted state: guidelines for writing entities [check-state utility]

[Svetoslav Neykov <sv...@cloudsoftcorp.com>]

Aled,

check-state utility is a great idea. HOT_BACKUP is very useful but still hard to integrate in scripting env (for example CI, upgrade scripts, etc.). Also HOT_BACKUP updates the persistence store with it's state which is sometimes undesirable (i.e. older masters failing because of the unknown node state). Perhaps we should we make HOT_BACKUP run in stealth mode?

For (2) I agree that validating against the XML is easier, though there could be some cases where we need Java state. I suggest that we integrate this in the persist path as well so developers get early feedback of potential problems.
Integrating in XStream should be fairly easy when object access is needed, no need to walk the object hierarchy manually.

---
I think we should promote using value objects for persisting. This makes the persisting surface explicit, makes it easier to upgrade. Also forces the developer to think about rebind. At the moment there's no good visibility into what get's pulled into the XML - I had a case where a classloader was being serialized which should've been caught much earlier.

Svet.


> On 11.03.2015 г., at 17:51, Aled Sage <al...@gmail.com> wrote:
> 
> All (especially Svet!),
> 
> I'm looking at adding a `brooklyn check-state --persistenceDir ...` utility.
> 
> This would do two things:
> 
> 1. make sure that the state is rebindable;
> 2. make sure the state follows best practices (e.g. no non-static
>   classes, no inner classes, avoid use of @SetFromFlag where possible,
>   etc)
> 
> For (1), I hope we can use the HOT_BACKUP code-path to check this.
> 
> For (2), there are a few possibilities. We could either run a set of checks against the persisted Strings (i.e. against the XML itself). Or we could de-serialize the objects and run a set of checks against that.
> 
> I think I favour validating against the XML - we can search for particular patterns. If doing it against the Java objects, we'd need to reflectively walk the object hierarchy to see if anything looks bad.
> 
> Thoughts/opinions?
> 
> Aled
> 
> On 10/03/2015 17:29, Aled Sage wrote:
>> All,
>> 
>> I've made some improvements to the docs at [1], to describe best practices. Any review / feedback extremely welcome.
>> 
>> Aled
>> 
>> [1] https://github.com/apache/incubator-brooklyn/pull/548
>> 
>> 
>> On 02/03/2015 15:43, Aled Sage wrote:
>>> Hi all,
>>> 
>>> We could do with better guidelines for folk writing entities, so that it produces "good" persisted state.
>>> 
>>> By "good", I mean that it will persist and rebind successfully, and that persisted state will not be overly affected by code changes in future versions of the relevant entities.
>>> 
>>> I suggest we add to the section "Writing Persistable Code" in [1], and that we move this to somewhere more prominent.
>>> 
>>> ---
>>> I wanted to get the community's input on the overall approach, and any thoughts on the guidelines.
>>> 
>>> We can switch to PR reviews/comments when it gets into the low-level details.
>>> 
>>> _*Current implementation*_
>>> We use xstream to persist/deserialize the Java objects as XML. For an entity, we persist its attributes and config. We also persist the policies, enrichers and feeds associated with the entity. We also persist locations + catalog.
>>> 
>>> The XML serialized form includes the class name and also the field names as the XML tags.
>>> 
>>> We may switch to json for the persistence format. This would give two benefits:
>>> 
>>> * Better persistence to things like MongoDB and CouchBase.
>>> * Move towards aligning with our YAML format (used for blueprint
>>>   definition), for improved consistency; eventually persisted state
>>>   could be deployed to a new Brooklyn!
>>> 
>>> 
>>> _*Additional guidelines*_
>>> 
>>> 1. Never use anonymous inner classes.
>>>   reason: the MyOuterClass$1 auto-generated name is really brittle,
>>>   for if the code is changed in the future.
>>> 2. Never use non-static inner classes.
>>>   reason: the outer class needs to be serialized as well.
>>> 3. Always use sensible field names (and use `transient` whenever you
>>>   don't want it persisted).
>>>   reason: the field name will be part of the persisted state.
>>> 4. Consider using Value Objects for persisted values.
>>>   reason: can give clearer separation of responsibilities, and clearer
>>>   control of what fields are being persisted (making backwards
>>>   compatibility simpler).
>>> 5. Consider writing transformers to handle backwards-incompatible code
>>>   changes.
>>>   reason: Brooklyn supports applying transformations to the persisted
>>>   state, which can be done as part of an upgrade process.
>>> 
>>> 
>>> _*Additional features?
>>> *_There are a few additional features we could consider supporting:
>>> 
>>> 1. Enforcement of guidelines
>>>   e.g. have a CLI option for `brooklyn check-state --strict`. This
>>>   would ensure guidelines are followed and that rebind will be possible.
>>> 2. Improved error reporting, including in web-console, when there are
>>>   rebind problems.
>>> 3. Support toJson() / fromJson() on objects being serialized, so that
>>>   folk can control their own custom serialization.
>>> 4. Support JavaBeans styles (i.e. persisting based on the getter method
>>>   name, rather than the field names).
>>> 
>>> For (3) and (4) I'm hesitant for us to support more stuff (more ways of doing things) unless it's going to genuinely be of major benefit to users.
>>> 
>>> Aled
>>> 
>>> [1] https://brooklyn.incubator.apache.org/v/latest/ops/persistence/index.html
>>> 
>>> 
>> 
>